-
Notifications
You must be signed in to change notification settings - Fork 60
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
Feature: Custom modal type #1507
Conversation
Hi @enkelmedia Thanks for this concept, it might be interesting for us as well when we start to looking into Tours. Only thing that strike me is that we in general dont call things How does that sound to you :-) |
Thanks for the feedback! Glad you found it interesting. Great point with the I was thinking about this now and thought that maybe this method could be confused with the way that the "mounted view" inside the modal is configured (I mean in the manifest for of type 'modal'). But then I thought that in the context of setting up the token (configure type, size etc.) makes it quite clear that we're configuring the modal it self and not the "mounted element". So given that I'm quite happy with just About the way to "point out" the element I guess that it would be nice if this was done in the same way as other elements, are we thinking about the Something also need to create the child Do you happen to have any pointers as to were to look for examples of how this is done in other places? Happy to update the PR, rename the method and adjust how the element is configured. |
Hello @enkelmedia |
@iOvergaard Will do =D |
I've been looking at this and kind of not sure which direction to take this. Since @nielslyngsoe requested the
I started to go down the path of using I ended up adjusting the I'm not sure if this was the kind of change the @nielslyngsoe was requesting or if he just referred to an actual element-instance or a string-element name? If we don't need the I might also lean towards naming the property I'll push the updates here for you to review. |
I think this is the way to go by making it async, @enkelmedia. I think this is what Niels meant, as that matches other areas of the Backoffice. "Element" is what we call properties like this in other areas of the Backoffice as well. I stumbled upon the modal type, you added ( |
@iOvergaard, nice that you think I made the right choice :) I'm happy to make adjustments based on any feedback from @nielslyngsoe - just let me know :) // m |
# Conflicts: # src/packages/core/modal/component/modal.element.ts
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.
Great!
This addresses umbraco/Umbraco-CMS#16677 to be able to support custom "modal host" elements. That is not the content of the modal but the modal it self. I figured I'll implement something to show how I think it could be done.
I also added a working example in the
/examples/
folder.Description
This PR will make it possible for extensions (and the core) to extend
UUIModalElement
to create a custom modal element.This is made possible by the added
elementFactory
parameter that is called bymodal.element
when the modal is created.Types of changes
Motivation and context
This makes it possible to extensions and core developers to create other types of modals while still reusing the foundation with the core Modal Manager Context, routing etc.
How to test?
There is a example included in the PR, to try it out you would need to find a way to open the modal.
In some element add code like this:
Let me know what you think and if there is any adjustments you would like me to make.
Cheers!