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

Feature/Proposal: Added support for element factory for modal manager context #1507

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

enkelmedia
Copy link
Contributor

@enkelmedia enkelmedia commented Mar 31, 2024

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 by modal.element when the modal is created.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (minor updates related to the tooling or maintenance of the repository, does not impact compiled assets)

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:

#modalContext?: UmbModalManagerContext;

constructor() {
	super();

	this.consumeContext(UMB_MODAL_MANAGER_CONTEXT,(instance)=> {
	      this.#modalContext = instance;
        });
}

onClickModal(){

	const modalContext = this.#modalContext?.open(this, EXAMPLE_MODAL_TOKEN, { value : {
		text : 'modal text'
	}});

	modalContext
		?.onSubmit()
		.then((e) => {
			console.log('Modal returened:', e.text)
		})
		.catch(() => {
			console.log('Modal rejected')
		});
}

Let me know what you think and if there is any adjustments you would like me to make.

Cheers!

@nielslyngsoe
Copy link
Member

nielslyngsoe commented Apr 2, 2024

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 Factory. And actually having a prop that accepts either a element-name or a method that returns an element is something we have in a few places. (I know that this one does not accept a element-name of now, but I think it would make sense that it could)
And in these place we do not use this term, so could we remove factory and just call it element or if that potentially becomes confusing then modalElement?

How does that sound to you :-)

@enkelmedia
Copy link
Contributor Author

Thanks for the feedback! Glad you found it interesting.

Great point with the factory part, I did not put a lot of effort in the naming.

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 element and then make sure to add comments/docs to clarify.

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 element: ()=>import('./yada-yada.js')-approach? I guess it would be possible to set constrains so that only a element that extends UUIModalElement can be used? Not sure how.

Something also need to create the child uui-dialog, this done in the "factory" right now, I guess we could move that into the #createContainerElement() method of modal.element.ts?

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.

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