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

feat(dialogs): Make dialogs look uniform #640

Closed
wants to merge 53 commits into from
Closed

feat(dialogs): Make dialogs look uniform #640

wants to merge 53 commits into from

Conversation

tomfrenzel
Copy link

@tomfrenzel tomfrenzel commented Apr 21, 2020

Every Dialog now has the same appearance

Steps how a Dialog now is being opened:

  1. The 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.
  2. The Service now opens the Dialog using the DialogsComponent as Template and passes the Dialog Configuration (such as Size, Title, Content, and Additional Data).
    As Content, a Component is being set.
    2020-04-22 08-06-42_Trim

Layout of the Dialog

Screenshot (9)
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

  1. The Profile picture enlargement dialog hast to be tested since I wasn't able to do so on my local environment.
  2. The current method using a Switch Statement to open the different dialogs isn't the most elegant way to do so. Let me know if you have any ideas on how to open the Dialog and pass its parameters properly or just make commits by yourself

Closes #411

@tomfrenzel tomfrenzel added feature An enhancement or a new functionality feature. help wanted Extra attention is needed labels Apr 21, 2020
@T-Systems-MMS T-Systems-MMS deleted a comment from allcontributors bot Apr 21, 2020
@tomfrenzel
Copy link
Author

@all-contributors please add @tomfrenzel for code

@allcontributors
Copy link
Contributor

@tomfrenzel

I've put up a pull request to add @tomfrenzel! 🎉

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-640.demo-phonebook.me

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-640.demo-phonebook.me

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-640.demo-phonebook.me

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-640.demo-phonebook.me

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-640.demo-phonebook.me

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-640.demo-phonebook.me

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-640.demo-phonebook.me

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-640.demo-phonebook.me

@mschwrdtnr
Copy link
Member

should the code rather moved to the folder /shared @DanielHabenicht ?

@tomfrenzel
Copy link
Author

@DanielHabenicht @mschwrdtnr would you review my code again so this PR can be merged

@mmsgithub-ci
Copy link
Collaborator

Preview Environment ready at https://pr-640.demo-phonebook.me

@DanielHabenicht
Copy link
Collaborator

should the code rather moved to the folder /shared @DanielHabenicht ?

yep I think so. There is even an existing folder shared/dialogs

@DanielHabenicht
Copy link
Collaborator

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?
And settings sensible defaults in MAT_DIALOG_DEFAULT_OPTIONS

Let's have a chat before continuing.

@tomfrenzel
Copy link
Author

sounds good

@paule96
Copy link
Collaborator

paule96 commented Sep 8, 2020

this PR will be closed because we start with a new architecture. (see last comment of daniel)

@paule96 paule96 closed this Sep 8, 2020
@paule96
Copy link
Collaborator

paule96 commented Sep 8, 2020

this PR will be closed because we start with a new architecture. (see last comment of daniel)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature An enhancement or a new functionality feature. help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom "actions footer" to the bottom of the Dialog template Take dialogues to the same level
6 participants