-
Notifications
You must be signed in to change notification settings - Fork 13
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
feat(dialogs): Make dialogs look uniform #640
Conversation
Release Notes are now displayed inside the new Dialog component
Dialogs now take a component as input instead of a text or HTML file
That allows a child component to close the dialog
-bug-report-consent and notifacation component are now handled by Dialog -removed feedback-drawer module
Co-authored-by: Restyled.io <[email protected]>
@all-contributors please add @tomfrenzel for code |
I've put up a pull request to add @tomfrenzel! 🎉 |
Preview Environment ready at https://pr-640.demo-phonebook.me |
Co-authored-by: Restyled.io <[email protected]>
Preview Environment ready at https://pr-640.demo-phonebook.me |
Preview Environment ready at https://pr-640.demo-phonebook.me |
Preview Environment ready at https://pr-640.demo-phonebook.me |
Preview Environment ready at https://pr-640.demo-phonebook.me |
Preview Environment ready at https://pr-640.demo-phonebook.me |
Preview Environment ready at https://pr-640.demo-phonebook.me |
Preview Environment ready at https://pr-640.demo-phonebook.me |
should the code rather moved to the folder /shared @DanielHabenicht ? |
@DanielHabenicht @mschwrdtnr would you review my code again so this PR can be merged |
Preview Environment ready at https://pr-640.demo-phonebook.me |
yep I think so. There is even an existing folder |
Overall, I think to split a dialog into different components (header, footer, main) makes it really difficult to program a dialog, as you have to think about handling events between those. (and as you may have already experienced). Also Bundling every dialog into the service loads every dialog at startup, which loads other lazy-loaded components. Which effectively runs lazy loading useless. Also, this shifts complexity only to a central service, which effectively does the same as before but centralized. (e.g. instead of referencing the component, you now reference a string - essentially the same). What about an easier approach of just using a template (via Content-projection) for each dialog which enforces the layout? Let's have a chat before continuing. |
sounds good |
this PR will be closed because we start with a new architecture. (see last comment of daniel) |
this PR will be closed because we start with a new architecture. (see last comment of daniel) |
Every Dialog now has the same appearance
Steps how a Dialog now is being opened:
displayDialog()
method, which is located inside the Dialog Service, gets called.Currently, you'll have to add a parameter to
displayDialog()
that is being handled by a switch Statement.DialogsComponent
as Template and passes the Dialog Configuration (such as Size, Title, Content, and Additional Data).As Content, a Component is being set.
Layout of the Dialog
The Dialog component (red) serves as the Template. It Includes title, close button, and a view container (orange). The view container displays the component which was set inside the Service.
What needs to be done/changed
Closes #411