Post Summary Panel: Restore height:auto for toggle buttons
#65362
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] Bug
t-hamano
commented
Sep 16, 2024
Comment on lines
+8
to
+11
| .editor-post-status__toggle.editor-post-status__toggle { | ||
| padding-top: $grid-unit-05; | ||
| padding-bottom: $grid-unit-05; | ||
| } |
There was a problem hiding this comment.
In order to override the padding on the button component, we need to increase the CSS specificity to 0-3-0.
|
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: +28 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
jameskoster
approved these changes
Sep 16, 2024
There was a problem hiding this comment.
This fixes the issue.
It's a bigger discussion but I wonder if it should be addressed upstream... should the Button component use min-height rather than height? cc @mirka.
I was wondering the same thing. Do you know how many places outside of the post summary panel there are where we might need to override the fixed height? |
|
Not off the top of my head, but it seems responsible for the Button component to elegantly scale to accommodate longer labels regardless. Not just for consumer use, but also because there could be cases where i18n makes a core button label much longer to the point where it wraps. |
|
Thank you for your feedback. I would like to merge this PR to prevent this issue from occurring in WP 6.7, but as you say, it might be an issue that should be resolved further upstream. |
That would be ideal, in addition to lowering specificity on some styles as well. The only problem is that it's a pretty big endeavor given the surface area for breakage 😅 Maybe if we find a good opportunity to put in the elbow grease! |
|
I'll open an issue to track separately. |
|
@jameskoster Thank you for submitting #65401. For now, I believe this PR is the best approach to address the issue and I'd like to merge it. |
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.
Related to #63658
What?
This PR changes the height of the toggle buttons in the post summary panel from a fixed
32pxtoauto. This fixes a layout issue that occurred when the button text was long:Why?
#63658 removed the
height:auto;for the toggle button, so the height was fixed at 32px. This is fine when there is little text, but when it is long, it causes text overflow.How?
The height of the toggle button cannot be fixed, so restore
height: auto;. This makes the size of the post status button with a24pxicon36px.To solve this, I considered the following two approaches:
8pxto4px.24pxto20px.I adopted the first approach, which does not cause any visual changes.
Testing Instructions