-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Button: Migrate to text-align block support #73732
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to add a "migration hook" to the edit function (like we did in the paragraph block) for folks that toggle the old attributes dynamically use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,7 +21,6 @@ export default function save( { attributes, className } ) { | |||||||
| const { | ||||||||
| tagName, | ||||||||
| type, | ||||||||
| textAlign, | ||||||||
| fontSize, | ||||||||
| linkTarget, | ||||||||
| rel, | ||||||||
|
|
@@ -46,7 +45,6 @@ export default function save( { attributes, className } ) { | |||||||
| borderProps.className, | ||||||||
| typographyProps.className, | ||||||||
| { | ||||||||
| [ `has-text-align-${ textAlign }` ]: textAlign, | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be considered a breaking change if themes or extenders rely on this class.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The gutenberg/packages/block-editor/src/hooks/use-typography-props.js Lines 48 to 50 in 2dbaeb1
|
||||||||
| // For backwards compatibility add style that isn't provided via | ||||||||
| // block support. | ||||||||
| 'no-border-radius': style?.border?.radius === 0, | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { useEvent } from '@wordpress/compose'; | ||
| import { useEffect, useRef } from '@wordpress/element'; | ||
| import deprecated from '@wordpress/deprecated'; | ||
| import { useDispatch } from '@wordpress/data'; | ||
| import { store as blockEditorStore } from '@wordpress/block-editor'; | ||
|
|
||
| /** | ||
| * If a plugin is still using the old textAlign attribute, we need to migrate its value | ||
| * to the new style.typography.textAlign attribute. | ||
| * | ||
| * @param {Object} props Block props. | ||
| */ | ||
| export default function useDeprecatedTextAlign( props ) { | ||
| const { name, attributes, setAttributes } = props; | ||
| const { textAlign } = attributes; | ||
| const { __unstableMarkNextChangeAsNotPersistent } = | ||
| useDispatch( blockEditorStore ); | ||
| const updateStyleWithAlign = useEvent( () => { | ||
| deprecated( `textAlign attribute in ${ name }`, { | ||
| alternative: 'style.typography.textAlign', | ||
| since: '7.0', | ||
| } ); | ||
| __unstableMarkNextChangeAsNotPersistent(); | ||
| setAttributes( ( currentAttr ) => ( { | ||
| style: { | ||
| ...currentAttr.style, | ||
| typography: { | ||
| ...currentAttr.style?.typography, | ||
| textAlign, | ||
| }, | ||
| }, | ||
| } ) ); | ||
| } ); | ||
| const lastUpdatedAlignRef = useRef(); | ||
| useEffect( () => { | ||
| if ( textAlign === lastUpdatedAlignRef.current ) { | ||
| return; | ||
| } | ||
| lastUpdatedAlignRef.current = textAlign; | ||
| updateStyleWithAlign(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could use a new setter method for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you! I tried to adjust it, how does this look? 8a32ace
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what I had in mind, though it's not a blocker; I just wanted to share. Example patch
diff --git a/packages/block-library/src/utils/deprecated-text-align-attributes.js b/packages/block-library/src/utils/deprecated-text-align-attributes.js
index 0daa1405bd..4dd591f987 100644
--- a/packages/block-library/src/utils/deprecated-text-align-attributes.js
+++ b/packages/block-library/src/utils/deprecated-text-align-attributes.js
@@ -1,7 +1,6 @@
/**
* WordPress dependencies
*/
-import { useEvent } from '@wordpress/compose';
import { useEffect, useRef } from '@wordpress/element';
import deprecated from '@wordpress/deprecated';
import { useDispatch } from '@wordpress/data';
@@ -18,7 +17,12 @@ export default function useDeprecatedTextAlign( props ) {
const { textAlign } = attributes;
const { __unstableMarkNextChangeAsNotPersistent } =
useDispatch( blockEditorStore );
- const updateStyleWithAlign = useEvent( () => {
+ const lastUpdatedAlignRef = useRef();
+ useEffect( () => {
+ if ( textAlign === lastUpdatedAlignRef.current ) {
+ return;
+ }
+ lastUpdatedAlignRef.current = textAlign;
deprecated( `textAlign attribute in ${ name }`, {
alternative: 'style.typography.textAlign',
since: '7.0',
@@ -33,13 +37,10 @@ export default function useDeprecatedTextAlign( props ) {
},
},
} ) );
- } );
- const lastUpdatedAlignRef = useRef();
- useEffect( () => {
- if ( textAlign === lastUpdatedAlignRef.current ) {
- return;
- }
- lastUpdatedAlignRef.current = textAlign;
- updateStyleWithAlign();
- }, [ textAlign, updateStyleWithAlign ] );
+ }, [
+ __unstableMarkNextChangeAsNotPersistent,
+ name,
+ setAttributes,
+ textAlign,
+ ] );
} |
||
| }, [ textAlign, updateStyleWithAlign ] ); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
A lot of these deprecated versions will become unnecessary after #73116 so I think this is ok for now. and I do think there's a lot of benefits to this unification. (Global Styles support being the important one)