Fix permalink input field text overflow ellipsis for Firefox #57310
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
afercia
added
[Type] Bug
|
Size Change: -3 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 1a2c399. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7289236186
|
t-hamano
reviewed
Dec 24, 2023
There was a problem hiding this comment.
I was able to confirm this strange behaviour with Firefox on Windows OS as well. It seems that reducing the top and bottom padding can solve this problem. There seems to be no problem with the top and bottom padding being zero.
I don't have a strong opinion, but how about keeping the left and right padding at their original values? Regarding this point, I would like to ask @richtabor and @jasmussen their opinions.
I don't have a strong opinion either but to me the current left and right padding seems unnecessarily too big. There's large inconsistency in the padding of the various input fields across the UI though, at the point that I woudlnt' know what the standard paddign is. I will create a separate issue. |
|
This PR is waiting for design feedback since almost two weeks. I will wait one day more, then I'll proceed as to not block a clear improvement. Further adjustments can always be done later. |
|
Hi, happy new year. Just starting up here after some end of the year AFK. Change seems fine for now. |
t-hamano
reviewed
Jan 8, 2024
There was a problem hiding this comment.
I think we can ship this PR, but what about changing the value of padding to 0 $grid-unit-15? Because it's the top and bottom padding that is causing this problem.
Otherwise, a regression could occur if this padding value were changed back from $grid-unit-15 to $grid-unit-20 as part of #57310.
@t-hamano maybe you meant to link to another PR? 57310 this PR 🙂 Re: this PR, I'd agree that ideally the |
Sorry, I meant #57394 😅 I don't think it's decided at this point what value to use for form element padding. In the future, we may decide to standardize the padding (left and right) of form elements to If the value is |
😅 Thanks @t-hamano I don't see much activity on #57394, unfortunately. Re: the padding value, I'm not sure what the reasoning behind having uneven left/right and top/bottom values is. To me, the uneven padding is just not nice to see. Screenshot:
Glad to add a comment. |
afercia
force-pushed
the
fix/permalink-readonly-input-field-text-overflow-ellipsis-firefox
branch
from
1a2c399 to
375d807
Compare
t-hamano
approved these changes
Jan 30, 2024
afercia
deleted the
fix/permalink-readonly-input-field-text-overflow-ellipsis-firefox
branch
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 to #53245
See comment #53245 (comment)
Ping: @WordPress/gutenberg-design
What?
The text overflow ellipsis in the post-publish panel permalink input field is broken in Firefox
Why?
Showing the ellipsis should work in all major browsers.
How?
Reduce the padding that prevents the ellipsis to work in Firefox.
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Screenshots before and after: