Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIO-9201: Fix DataTable in quick inline embed issues #177

Merged
merged 3 commits into from
Oct 23, 2024

Conversation

mikekotikov
Copy link
Contributor

@mikekotikov mikekotikov commented Oct 21, 2024

Link to Jira Ticket

https://formio.atlassian.net/browse/FIO-9201

Description

What changed?

Added possibility to pass DOM element relative to which the script should be inserted, since inline embed uses Shadow Root element and scripts should be applied inside of it to work.

Why have you chosen this solution?

Use this section to justify your choices

Breaking Changes / Backwards Compatibility

Use this section to describe any potentially breaking changes this PR introduces or any effects this PR might have on backwards compatibility

Dependencies

formio/premium

How has this PR been tested?

Manually, since premium module is needed to reproduce failing scenario

Checklist:

  • I have completed the above PR template
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • My changes generate no new warnings
  • My changes include tests that prove my fix is effective (or that my feature works as intended)
  • New and existing unit/integration tests pass locally with my changes
  • Any dependent changes have corresponding PRs that are listed above

@mikekotikov mikekotikov force-pushed the fix/FIO-9201_quick_inline_embed branch from a64df9c to 97bdca1 Compare October 22, 2024 06:29
Copy link
Contributor

@johnformio johnformio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need to update jsdocs

polling: boolean = false,
onload?: (ready: Promise<any>) => void,
) {
static requireLibrary(name: string, property: string, src: string | Array<string>, polling: boolean = false, onload?: (ready: Promise<any>) => void, rootElement?: HTMLElement ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update jsdocs with new parameter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

Comment on lines +113 to +121
if (rootElement) {
rootElement.insertAdjacentElement('afterend', element);
return;
}
const { head } = document;
if (head) {
head.appendChild(element);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this attach the element twice?

Copy link
Contributor Author

@mikekotikov mikekotikov Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, since there's a flag in requireLibrary function hasResourceBeenAdded, that would prevent attaching the resource twice

@johnformio johnformio merged commit 47c1e0e into master Oct 23, 2024
8 checks passed
lane-formio pushed a commit that referenced this pull request Oct 29, 2024
* FIO-9201: Fix quick inline embed issues

* FIO-9201: Add small fix

* FIO-9201: Add new parameter into jsdocs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants