Improve modal and fullscreen/starter content view #55525
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
richtabor
added
[Type] Enhancement
|
Size Change: +89 B (0%) Total Size: 1.69 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 5ab7532. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6592901509
|
|
Definitely good to fix that visual bug with the search input 👍 I have a small preference for the old spacing around the modal though. The overlap with the document toolbar helps to reinforce the separation between modal / page. The new values create a little bit of tension between the W button and the top-left corner of the modal. Not a strong opinion though, if someone else feels the change is good I wouldn't object! |
I feel the same way. The previous spacing feels more fullscreen to me when the full header/toolbar/W isn't showing. |
Feel that, but I do think it feels better not having the header toolbar items visually cut by the modal, while also increasing the click area to quickly exit the modal. Makes the modal feel more connected to the experience, not so obtrusive. |
|
Good stuff.
&
Agree with both of the above, but to avoid a tangent (the situation where two or more design elements, such as lines, shapes, or objects, come into contact in a way that creates a visual or physical connection without a clear distinction or separation) I think we should either keep the padding as it was, or make it larger than 60px to clearly delineate that the modal doesn't have a relationship with the top toolbar. It could be enough to have it be 80px. That said, and this is related to the conversation here, around default modal sizes. To avoid the proliferation of local overrides (which can be fine), I appreciate that you are making this change directly in the modal component itself. But that's also to say, probably add a changelog entry. Nice! |
|
Personally I would just leave it as it is. Top layer should not interact in any way with the below layer. |
|
It looks like there's a lot going on here
|
|
I'll split out the winners in this experiment into separate PRs; thanks for the 👀 folks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Tweaks to make the fullscreen modal display better, as well as the starter content modal. Follow-up would include consolidating the template/template part modals (without so we don't have to duplicate styles.
Testing Instructions
Visuals