Try: Fix double-border. #61318
Closed
Try: Fix double-border. #61318
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
jasmussen
added
the
[Type] Regression
|
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: -2 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 4127574. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8922669302
|
|
Could we simplify the css quite a bit by removing the top border and negative margin? I'm not seeing any scenarios where the top border is needed 🤔 |
|
I believe the Panel component is designed to have borders all around so that it works in a standalone capacity. If we removed the top border, it would be a local change. Not a strong opinion, but @WordPress/gutenberg-components might know better. |
t-hamano
reviewed
May 2, 2024
There was a problem hiding this comment.
The Panel component is used in various scenarios, for example:
- Only one
Panelcomponent - Multiple
Panelcomponents are stacked Panelcomponents and non-Panel components are mixed and stacked
In all of these cases, I believe the current implementation is done to always display a 1px border above and below the Panel component.
If we always add a top negative margin to the component or remove borders, we may experience a one-pixel shift or the border may disappear in some situations.
What do you think of this alternative approach? It doesn't change the behavior of the components and focuses only on the site editor sidebar.
This approach is similar to what is done in the post editor.
diff --git a/packages/edit-site/src/components/sidebar-edit-mode/settings-header/style.scss b/packages/edit-site/src/components/sidebar-edit-mode/settings-header/style.scss
index d74432451e..4e0673c10a 100644
--- a/packages/edit-site/src/components/sidebar-edit-mode/settings-header/style.scss
+++ b/packages/edit-site/src/components/sidebar-edit-mode/settings-header/style.scss
@@ -1,6 +1,7 @@
.components-panel__header.edit-site-sidebar-edit-mode__panel-tabs {
padding-left: 0;
padding-right: $grid-unit-20;
+ margin-bottom: -1px;
.components-button.has-icon {
padding: 0;
diff --git a/packages/edit-site/src/components/sidebar-edit-mode/style.scss b/packages/edit-site/src/components/sidebar-edit-mode/style.scss
index 186908c88a..fbbde485dc 100644
--- a/packages/edit-site/src/components/sidebar-edit-mode/style.scss
+++ b/packages/edit-site/src/components/sidebar-edit-mode/style.scss
@@ -103,7 +103,3 @@
}
}
}
-
-.edit-site-sidebar__panel {
- margin-top: -1px;
-}
|
That makes sense. I can try that tomorrow, otherwise feel free to push changes. |
|
I took a different angle here: #61331 |
|
I believe that the problem is that with the sticky position for Switching that diff --git a/packages/interface/src/components/complementary-area-header/style.scss b/packages/interface/src/components/complementary-area-header/style.scss
index 20fbe881d5..d21d1bc314 100644
--- a/packages/interface/src/components/complementary-area-header/style.scss
+++ b/packages/interface/src/components/complementary-area-header/style.scss
@@ -41,6 +41,6 @@
// since the first panel is hidden.
.components-panel__header + .interface-complementary-area-header {
@include break-medium() {
- margin-top: 0;
+ top: 0;
}
}``` |
|
Excellent feedback, thanks everyone. Across the differing suggestions and #61331 (which I gave a ✅ and also fixes this), I'm not sure of the best path forward. What do you think? Happy to close this one. Also happy to have others push directly to this branch. |
|
Agreed we can close this in favor of #61331. |
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?
Fixes #61055.
There's a double-border above the first panel in the inspector now:
This PR fixes it so it's only a single border, as expected:
Why?
The panel component has a border all around. A negative margin is applied to make them stack visually with 1 border separation regardless.
How?
This PR makes it target all the panels, not just the first one.
Please note, there was probably a good reason why this wasn't the case previously, so it would be good to check every panel that uses the panel component, to see that things look right.
Testing Instructions
Open post, page, site editors, and check the first panel in the first tab of the inspector. There should only be a 1px border between tabs and content.