Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve scoping of separator class #6010

Merged
merged 4 commits into from
May 20, 2024
Merged

Improve scoping of separator class #6010

merged 4 commits into from
May 20, 2024

Conversation

JeffersonBledsoe
Copy link
Member

The .separator class, used in the blocks toolbar like shown below, has a really low specificity, causing it to leak (try creating a block named separator and seeing what happens to the blocks list 😉 ). This PR scopes it to .block .toolbar .separator, which is really it's intended location

Copy link

netlify bot commented May 9, 2024

Deploy Preview for volto failed. Why did it fail? →

Name Link
🔨 Latest commit 7bf1fbe
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/6645c809a2b91c0008542df9

Copy link

netlify bot commented May 9, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit f396812
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/664b2f7d4d08780008ac27b8

@@ -318,6 +318,16 @@ body.has-toolbar.has-sidebar-collapsed .ui.wrapper > .ui.inner.block.full {
0 2px 4px rgba(0, 0, 0, 0.05);
transform: translate(-50%, 0);

.separator {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JeffersonBledsoe not quite, the html block on the edit page also uses this class


html-block

I am ok with changing modifying the separator scope and/or prefixing it with other names so that it's not simply .separator.
I'm a bit surprised that on the demo site it's not prefixed .block .separator but even then it might conflict with a separator block

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ichim-david This is the toolbar that is mentioned and a screenshot taken of in the description. There's probably addons that use this CSS class too, but this is the only place I could find it in core

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JeffersonBledsoe you are correct, I did a better search and there are other separators such as within slate or the plone components but they have their own extra identifiers so this work is good.

@ichim-david ichim-david self-requested a review May 10, 2024 16:06
@sneridagh sneridagh merged commit 015658f into main May 20, 2024
69 of 70 checks passed
@sneridagh sneridagh deleted the separator-class-scoping branch May 20, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants