-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
add color support to separator block #16784
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
0b4c44d
add color support to separator block
senadir ce2d57a
add color to frontend render
senadir 3cba3fd
add support for dots style in frontend
senadir 09fc25b
fix review problems, update block.json file
senadir 026bb0b
fix border issue with theme.scss
senadir a815b5b
extend border remove in editor
senadir ad6f48c
fix problems with no color selected
senadir File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,12 @@ | ||
{ | ||
"name": "core/separator", | ||
"category": "layout" | ||
"category": "layout", | ||
"attributes": { | ||
"color": { | ||
"type": "string" | ||
}, | ||
"customColor": { | ||
"type": "string" | ||
} | ||
} | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,49 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import classnames from 'classnames'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { HorizontalRule } from '@wordpress/components'; | ||
import { | ||
InspectorControls, | ||
withColors, | ||
PanelColorSettings, | ||
} from '@wordpress/block-editor'; | ||
|
||
export default function SeparatorEdit( { className } ) { | ||
return <HorizontalRule className={ className } />; | ||
function SeparatorEdit( { color, setColor, className } ) { | ||
return ( | ||
<> | ||
<HorizontalRule | ||
className={ classnames( | ||
className, { | ||
'has-background': color.color, | ||
[ color.class ]: color.class, | ||
} | ||
) } | ||
style={ { | ||
backgroundColor: color.color, | ||
color: color.color, | ||
} } | ||
/> | ||
<InspectorControls> | ||
<PanelColorSettings | ||
title={ __( 'Color Settings' ) } | ||
colorSettings={ [ | ||
{ | ||
value: color.color, | ||
onChange: setColor, | ||
label: __( 'Color' ), | ||
}, | ||
] } | ||
> | ||
</PanelColorSettings> | ||
</InspectorControls> | ||
</> | ||
); | ||
} | ||
|
||
export default withColors( 'color', { textColor: 'color' } )( SeparatorEdit ); |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,41 @@ | ||
export default function save() { | ||
return <hr />; | ||
/** | ||
* External dependencies | ||
*/ | ||
import classnames from 'classnames'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { | ||
getColorClassName, | ||
} from '@wordpress/block-editor'; | ||
|
||
export default function separatorSave( { attributes } ) { | ||
const { | ||
color, | ||
customColor, | ||
} = attributes; | ||
|
||
// the hr support changing color using border-color, since border-color | ||
// is not yet supported in the color palette, we use background-color | ||
const backgroundClass = getColorClassName( 'background-color', color ); | ||
// the dots styles uses text for the dots, to change those dots color is | ||
// using color, not backgroundColor | ||
const colorClass = getColorClassName( 'color', color ); | ||
|
||
const separatorClasses = classnames( { | ||
'has-text-color has-background': color || customColor, | ||
[ backgroundClass ]: backgroundClass, | ||
[ colorClass ]: colorClass, | ||
} ); | ||
|
||
const separatorStyle = { | ||
backgroundColor: backgroundClass ? undefined : customColor, | ||
color: colorClass ? undefined : customColor, | ||
}; | ||
|
||
return ( <hr | ||
className={ separatorClasses } | ||
style={ separatorStyle } | ||
/> ); | ||
} |
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see there was some discussion about the use of
!important
here in the pull request comments, and separately a fix addressing a TwentyTwenty-specific incompatibility at Trac#47811.Which leaves the question: Do we actually need it? Can it be removed?
See also: #19539
Alternatively, I wonder:
.has-text-color
and/or.has-background
, representative of the fact that the default should only need to be reset when the user chooses to apply a custom color (correct me if I'm mistaken).has-text-color
and/or.has-background
as additional modifier classes:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if a different method for generating dots was used?
The current method is a lot of CSS, that is a hassle for themes to override. I was going for the minimal style of making dots with multiple background images using radial gradients, but the current style using !important wouldn't let me.
I also couldn't use a dotted border because all those other styles I would have to override.
So what if the core style is the radial gradient instead of an important no background?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be worth exploring a simpler approach.