Fix overflow in patterns filter container #65987
Open
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
madhusudhand
requested review from
getdave,
mikachan,
richtabor and
MaggieCabrera
|
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: +18 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
mikachan
added
[Package] Block editor
getdave
reviewed
Oct 9, 2024
Comment on lines
2
to
5
| margin-top: $grid-unit-10; | ||
| margin-left: -$grid-unit-10; | ||
| margin-right: -$grid-unit-10; | ||
| margin-bottom: -$grid-unit-10; |
There was a problem hiding this comment.
I'd like to understand a little more about why these margins were needed and why it's now ok to remove them without consequences 🙏
There was a problem hiding this comment.
Since, styles are separated, I am going to revert them back to original state.
mikachan
reviewed
Oct 9, 2024
There was a problem hiding this comment.
This fixes the scrollbar issue on the patterns filter menu for me 👍
| trunk | This PR |
|---|---|
It increases the spacing in case of tools menu, but they are now aligned with the icons. May be reduce the container padding here?
Perhaps we could add some specific padding to .components-dropdown__content .components-popover__content .block-editor-tool-selector__help, e.g. in these dropdown styles, we could add the following to reduce the padding around this text:
.components-dropdown__content {
.components-popover__content {
.block-editor-tool-selector__help {
padding: $grid-unit-20 $grid-unit-10;
}
...
Edit: We should avoid using the .block-editor-tool-selector__help class here - see this comment below - but I still think this might be a way forward (using a different selector) to avoid this style regression.
Pinging @jasmussen on this too, as it looks like you were working on this CSS back in #34435.
|
Just noting that I don't think components styles are allowed to be coupled to the block editor classes. The components package should be decouple from all "instances" where it might be used. |
Ah yes, that makes sense. In that case, if we did want to add some specific styles here, we'd need to target them in a different way to avoid using the block editor class. |
|
Nice to see just red code in this PR. To answer Dave's good question, those local CSS overrides piled up from a delicate balancing act chronicled in #34435 and #34378 (comment), and are ultimately a reflection of the lack of maturity of the components at the time. Best practice is to never have local CSS overrides, so the more we can remove, the better. |
Makes sense. Disconnected the components by creating own class for patterns filter in aea7072. Regarding the text itself, I think it deserves its own component such as |
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?
This fixes overflow in patterns filter by removing the negative margins.
@richtabor It increases the spacing in case of tools menu, but they are now aligned with the icons. May be reduce the container padding here?
Testing Instructions
+-> Patterns -> Filter and verify it has no scrollbars