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

Stack based Modal Navigation #1034

Closed
risalfajar opened this issue Feb 24, 2023 · 7 comments
Closed

Stack based Modal Navigation #1034

risalfajar opened this issue Feb 24, 2023 · 7 comments
Labels
feature request Request a feature or introduce and update to the project.
Milestone

Comments

@risalfajar
Copy link
Contributor

Describe what feature you'd like. Pseudo-code, mockups, or screenshots of similar solutions are encouraged!

Currently, the Modal system is based on Queue (FIFO) principle. But in terms of navigation, a Stack (LIFO) is more commonly used.

Also, with the current system, a currently showing modal can't show another modal unless it has been closed and all the modals have been called/triggered beforehand:

[1, 2, 3].forEach((dNum: number) => {
	const d: ModalSettings = {
		type: 'alert',
		title: `Modal ${dNum}`,
		body: `The modal body of ${dNum}.`
	};
	modalStore.trigger(d);
});

Transitioning to Stack based allows more control and customization, like:

  1. A modal can have a child modal or shows another modal without closing itself.
  2. Showing multiple modals at once is possible, if needed (by overriding Modal. svelte). This can be useful in some case since a Component Modal will lose its states when closed.

What type of pull request would this be?

Enhancement

Any links to similar examples or other references we should review?

No response

@risalfajar risalfajar added the feature request Request a feature or introduce and update to the project. label Feb 24, 2023
@endigo9740 endigo9740 added this to the Future/Whenever milestone Feb 24, 2023
@risalfajar
Copy link
Contributor Author

One suggested approach is to change modalStore.close() to use pop() instead of shift(), and replace any $modalStore[0] with $modalStore[$modalStore.length - 1] (or better, create a variable like $modalStore.current).
This is a breaking change though.

And for showing multiple modals at once, we can replace:

{#if $modalStore.length > 0}
    {#key $modalStore}

in Modal.svelte, with

{#each $modalStore as store, index (index)}

Happy to create a PR if approach is approved.

@saturnonearth
Copy link
Contributor

I would love to have overlapping modals. This is super useful if you want to alert the user when closing a modal, incase their work may be lost.

Examples...

image

image

Placement would be important too. You want to anchor to the top of existing modal or center, or even be outside in it's own global space.

To not introduce a breaking chance, could there be a way to add a setting between creating a queued modal, and overlapping modal? Default would be queued, so no breaking change.

@Beiri22
Copy link

Beiri22 commented May 9, 2023

I also need to trigger stacked Modals as described here. Would the following work?

// Original function pushes
/*
modalStore.trigger({
	type: 'component',
	component: {
		ref: DocumentComponent,
	}
});
*/
// Use the underlaying method update to manually put the new modal to the front?
modalStore.update(modals => [{
	type: 'component',
	component: {
		ref: DocumentComponent,
	}},
	...modals
])

Is seems to open the new modal and temporary close the old one. So it's not yet a real stacking.

@saturnonearth
Copy link
Contributor

It would not be a breaking change if it was implemented in a way that the que system was still the default, and a prop option to choose "stacked" would be available. Although I personally think stacking them should be the default, as having multiple modals lined up in a que kind of seems like a situation you would never want to put your end-user in anyways. I can't imagine a website that kept popping up modals when I tried to exit them - that would be pretty frustrating.

@endigo9740
Copy link
Contributor

Yeah for anyone wondering, I'm still brainstorming this. We would have to find a way that doesn't cause layout issues (lots of modals on a small screen) and doesn't frustrate the user as described above. It sometimes takes a while to circle back to this, but it has remained in our queue because we plan to revisit at some point in the future.

@endigo9740 endigo9740 self-assigned this Jun 2, 2023
@endigo9740 endigo9740 removed their assignment Aug 29, 2023
@rskvazh
Copy link

rskvazh commented Oct 17, 2023

Very requested feature. Simple use case: Delete button inside modal which should show a Prompt modal over it (or instead, but reopen previous if needed)

PS. For now I'll use this workaround #1034 (comment), but state of previous modal was resetting.

@endigo9740
Copy link
Contributor

In an effort to prepare for Skeleton v3, we're consolidate some related issues down to a single ticket. This will ensure that we can see the full context of requests when the time comes to refactor and update this feature going forward. If you wish to add additional feedback or suggestions, please so here:

@endigo9740 endigo9740 modified the milestones: v3.0 (Next), v2.0 Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request a feature or introduce and update to the project.
Projects
None yet
Development

No branches or pull requests

5 participants