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.

Sign up for GitHub

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a7db360
Improve scoping of separator class
JeffersonBledsoe May 9, 2024
d7f182f
Changelog
JeffersonBledsoe May 9, 2024
7bf1fbe
Merge branch 'main' into separator-class-scoping
JeffersonBledsoe May 16, 2024
f396812
Merge branch 'main' into separator-class-scoping
JeffersonBledsoe May 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/volto/news/6010.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed separator edit class spilling outside of the block toolbar @JeffersonBledsoe
20 changes: 10 additions & 10 deletions packages/volto/theme/themes/pastanaga/extras/blocks.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

volto/packages/volto/src/components/manage/Blocks/HTML/Edit.jsx

Line 295 in 09239bf

<div className="separator" />

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.

position: relative;
top: 5px;
display: inline-block;
height: 24px;
border-right: 1px solid #ddd;
margin: 0 0 4px 4px;
vertical-align: bottom;
}

form {
display: flex;
}
Expand Down Expand Up @@ -591,16 +601,6 @@ body.has-toolbar.has-sidebar-collapsed .ui.wrapper > .ui.inner.block.full {
height: 100%;
}

.separator {
position: relative;
top: 5px;
display: inline-block;
height: 24px;
border-right: 1px solid #ddd;
margin: 0 0 4px 4px;
vertical-align: bottom;
}

// HTML block
.html-editor {
z-index: 1;
Expand Down
Loading