-
Notifications
You must be signed in to change notification settings - Fork 55
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
Use identifiers on modal triggers and modal component instead of integral trigger #3541
Use identifiers on modal triggers and modal component instead of integral trigger #3541
Conversation
…components instead of having the trigger as integral part of the modal component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, don't have much to add 👍
Co-authored-by: ammar92 <[email protected]>
Co-authored-by: ammar92 <[email protected]>
…ger-from-component-structure
…ger-from-component-structure
…ger-from-component-structure
Re: making the dialog generic: it's currently responsible for managing form elements that are rendered inside of it. It might be better to leave that responsibility to the content itself, e.g. the JS for a specific form, e.g. via some sort of |
FYI: Marked as "Draft" because the current implementation on the plugin tile still needs refactoring. |
…ger-from-component-structure
…ger-from-component-structure
…mponent-structure' of https://github.com/minvws/nl-kat-coordination into feat/update-modal-component-by-removing-trigger-from-component-structure
…ger-from-component-structure
… a href="#" trigger use case
…ger-from-component-structure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stray console.log but other than that LGTM!
Co-authored-by: Peter-Paul van Gemerden <[email protected]>
…ger-from-component-structure
Checklist for QA:
What works:There do not appear to be any unexpected side effects. Can be merged. What doesn't work:n/a Bug or feature?:n/a |
…ger-from-component-structure
…ger-from-component-structure
…ger-from-component-structure
…mponent-structure' of https://github.com/minvws/nl-kat-coordination into feat/update-modal-component-by-removing-trigger-from-component-structure
Co-authored-by: Jan Klopper <[email protected]>
…ger-from-component-structure
Use identifiers on modal triggers and modal component, instead of having the trigger as integral part of the modal component.
Changes
Removed the trigger slot and added some logic to assign identifiers to both triggers and modals. This way we are free to place our triggers and modals separately, providing more flexibility when implementing. Needed for #3379, for example.
Issue link
Enabler for #3379
Demo
N/a: the changes aren't visible.
QA notes
Verify that implementing the component in a HTML template by following the README will result in a working trigger-modal pair
Code Checklist
.env
changes files if required and changed the.env-dist
accordingly.Checklist for code reviewers:
Copy-paste the checklist from the docs/source/templates folder into your comment.
Checklist for QA:
Copy-paste the checklist from the docs/source/templates folder into your comment.