URLPopover: restore min-width style #59274
Merged
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
t-hamano
added
[Type] Regression
t-hamano
added
[Feature] UI Components
|
I will give this PR a backport label. This is because this layout problem appeared in #57608, and I think it can also be considered a regression that occurred in WP6.5. |
t-hamano
changed the title
t-hamano
force-pushed
the
url-popover-min-width
branch
from
3304352 to
9c264a8
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +17 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
jasmussen
reviewed
Feb 22, 2024
| @@ -58,6 +58,7 @@ | |||
| text-overflow: ellipsis; | |||
| white-space: nowrap; | |||
| margin-right: $grid-unit-10; | |||
| min-width: 150px; | |||
There was a problem hiding this comment.
Do you recall what the value was before?
I think the input field here is 300px. Maybe keep that?
There was a problem hiding this comment.
Do you recall what the value was before?
The previous value was 150px and I have restored the same value in this PR.
|
Here's what I'm seeing. Especially the dropdown chevron looks like it's in a poor state, with the separator there:
@richtabor since you've worked on this element, any instincts? |
|
Flaky tests detected in 9c264a8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8005088631
|
|
@jasmussen Sorry for the insufficient explanation. Below is a screencast in trunk. After entering the link, instead of pressing the "Edit" button, click the "Link settings" button. This PR solves this scenario. af1b922c789cbbe954f8d002e691a2cd.mp4 |
This issue should be resolved in #58906. we know I need to adjust the overall layout of this popover, but for WP6.5 I've broken it down into the smallest possible PRs. |
jasmussen
approved these changes
Feb 22, 2024
There was a problem hiding this comment.
Thanks for the added context.
We might want to see if we can use a 300px min-width keeps a more stable experience, ideally the width of the popover doesn't change between editing and resting states. If 300 fixes that, feel free to slipstream that in. Otherwise, happy to land this as a fix for the regression.
|
Thanks for the review!
If we simply change it from 150px to 300px, the short URL preview popover feels very long to me. In the future, when working on more general improvements, we may be able to consider the ideal approach again.
|
|
I just cherry-picked this PR to the cherry-pick-wp-6-5-beta-3 branch to get it included in the next release: eddf324 |
getdave
added
Backported to WP Core
9 tasks
getdave
pushed a commit
that referenced
this pull request
Feb 27, 2024
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.
Follow up: #57608
Related to #58741
What?
This PR enforces a minimum width for
URLPopovercomponents and fixes unintended layout collapse.Why?
In #57608, the
URLPopovercomponent was redesigned. At that time, the CSS that enforces the minimum width appears to be lost.How?
I simply restored the original value.
Testing Instructions
Screenshots or screencast
Note
The misalignment of the separator when opening the link settings will be fixed with #58906.