-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add a full screen modal component #28574
Conversation
Size Change: +3.95 kB (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
I understand how this component might benefit work in #28390 — it seems a legitimate use case. But although the following is pretty, I don't think it's actually an improvement over what we have: It's cleaner, but it severs the context you have by seeing the editor around it. It may be fine? But I'd love some more sanity checks, and happy to defer. But one path forward is to build this component for use with, as mentioned, 28390, and use it there, but not for preferences or keyboard shortcuts. Or does it replace entirely the current modal system? |
It is important to keep the context where one is when one clicks example the preferences/options, templates, etc etc. This makes full screen modals difficult because they shatter any contextual relation to the page/post the user is on. We could for instance add a similar method for templates. The context of a modal still keeps the user related to where they are. Modals can for instance take over most of the screen still keeping the outside area intact and semi greyed/blacked out where one can see the Gutenberg area and still keep a connection to where one came from. |
I quite like how iOS handles modals, it does a good job of maintaining context without sacrificing too much space or creating a cramped sensation. I made a prototype ages ago to see how it translated to desktop. The details probably need fine tuning, but I think it works quite well. Obviously scales down to mobile nicely as well :D |
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.
I really like this! Thanks Riad!
It's cleaner, but it severs the context you have by seeing the editor around it.
I echo Joen for not making the Preferences
full screen, but Keyboard shortcuts
seems much better fullscreen IMO.
Either way this can be merged without changing any existing component yet - though I really liked fullscreen Keyboard shortcuts :)
The only thing that seems just a bit off to me is the animation. I'm not sure if I'm just accustomed to the current one that is different.. 🤔
Thanks for your thoughts everyone, I removed the full screen modal design from the "preferences" modal. We can focus on the component for now in order to land it and use it in #28390 instead |
It looks really interesting @jameskoster The thing though with the existing small modal screen is that turning on specific options one would also right away see what happens in the screen. For instance using text instead of icons one would see what it looks like in the Gutenberg screen, like a preview, and then decide if one wants to use it or not. Meaning with options it is helpful to see as much of the Gutenberg screen as possible to preview clicking the various options turning them on or off. |
Oh, yes, ignore the contents of the modal there. I agree that preferences should remain in a "regular" modal. |
@jameskoster Happy to try to implement that if that's the direction we want to go and if it fits #28390 Can you share the custom CSS you had to write to make that work (even hacky one) |
It would be great if we can decide/define standard modals to use. Looking at the various types of modals in use today. So that we can create a consistency of visual items that popup on the screen. Such as for the options/preferences modal all the way up in size to the Add Media modal. (EDIT: I am just focusing on the visual/UI not the code behind it.) |
I think we shouldn't be using "Add Media" kind of modals at all, that's backbone modals which doesn't fit great the components library of Gutenberg. Ideally, we should do the opposite, at some point in the future, deprecate the media modal as it exists today and rely on |
Ah I'm afraid I closed the tab. For the "background" it was basically a scale transform on Then the modal itself had a Happy to dig in to the css if we can fix the branch up. Probably just needs a rebase. |
This is the issue here, the problem is that we don't really know that |
baf7e78
to
5a70576
Compare
Ok found a hack, let me know how it looks (try keyboard shortcuts) and feel free to tweak the styles and animations. |
Nice! I think there are a few visual optimisations we can make. I'll look into this on Monday 👍 |
Made some style and animation adjustments: modal.mp4It's a shame we can't have the animation reverse when the modal is closed. It is a little jarring to jump immediately back with no transition. |
Closing as we might not need this anymore in the latest template switching designs. |
This PR restores an experiment I did a long time ago to allow full screen modals.
The main use-case for this is going to be the "template switch" modal explored in #28390
For the current PR, I applied the styles to the "Preferences" modal as well. I personally like it but it folks think it's not a good fit, we can restore the style before merging.
Testing instructions