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

Quote: Rename the align attribute to textAlign #42960

Merged
merged 46 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
6edad09
Quote: Rename align attribute to textAlign
carolinan Aug 3, 2022
45a9863
Update deprecated.js
carolinan Aug 4, 2022
746c4fb
Update core__quote__deprecated-2.serialized.html
carolinan Aug 4, 2022
745786e
Merge branch 'trunk' into update/rename-align-attribute
carolinan Oct 4, 2022
976d99f
Add alignments for testing
carolinan Oct 4, 2022
67d16e7
Update fixtures, add align block support to the deprecation
carolinan Oct 5, 2022
11ad22b
Remove left/right/center alignments
carolinan Oct 6, 2022
9112eda
Copy save function to deprecation v4. Combine migrations in deprecati…
carolinan Oct 13, 2022
2def1cc
Merge branch 'trunk' into update/rename-align-attribute
carolinan Dec 30, 2022
ef50128
Merge branch 'trunk' into update/rename-align-attribute
carolinan Feb 1, 2023
5fd2a25
Merge branch 'trunk' into update/rename-align-attribute
carolinan Feb 2, 2023
a8057ff
Merge branch 'trunk' into update/rename-align-attribute
carolinan Feb 23, 2023
757a999
Merge branch 'trunk' into update/rename-align-attribute
carolinan Mar 6, 2023
beb1c9d
Update deprecated.js
carolinan Mar 6, 2023
46a7b01
Merge branch 'trunk' into update/rename-align-attribute
carolinan Mar 14, 2023
686ead6
Merge branch 'trunk' into update/rename-align-attribute
carolinan Apr 12, 2023
6367fe6
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jun 14, 2023
215d6fc
build docs
ntsekouras Jun 16, 2023
51e1813
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jun 19, 2023
547539d
Try removing compose, add isEligible
carolinan Jun 19, 2023
420beba
Update deprecated.js
carolinan Jun 19, 2023
46940c5
Update transforms.native.js.snap
carolinan Jun 19, 2023
2aa7d2e
Update index.native.js
carolinan Jun 20, 2023
ac28077
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jun 26, 2023
fa4566b
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jul 2, 2023
f2f96c5
fix: Paragraphs inherit parent text alignment (#52293)
dcalhoun Jul 7, 2023
48ef842
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jul 14, 2023
844482d
Merge branch 'trunk' into update/rename-align-attribute
carolinan Aug 17, 2023
e1d245a
Fixture for version with align attribute
carolinan Aug 21, 2023
54188a1
Revert the block support change
carolinan Aug 21, 2023
92a9b73
Update supoports in the deprecation
carolinan Aug 21, 2023
f3ef6e7
Merge branch 'trunk' into update/rename-align-attribute
carolinan Sep 27, 2023
8434f65
Try to fix the migration
carolinan Sep 27, 2023
c45e17a
Tests: Remove quote from currentlySupportedBlocks in isContainerRelated
carolinan Oct 2, 2023
6509edd
Merge branch 'trunk' into update/rename-align-attribute
carolinan Oct 2, 2023
ecd78fc
Update migration, apply changes requested during review.
carolinan Oct 10, 2023
e280bac
Merge branch 'trunk' into update/rename-align-attribute
carolinan Nov 20, 2023
f6a6f48
Merge branch 'trunk' into update/rename-align-attribute
carolinan Nov 23, 2023
49d93d5
Merge branch 'trunk' into update/rename-align-attribute
carolinan Nov 27, 2023
79d760c
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jan 3, 2024
57b74b7
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jan 8, 2024
7b1be6f
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jan 9, 2024
6245a17
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jan 12, 2024
1b76aba
Merge branch 'trunk' into update/rename-align-attribute
carolinan Jan 25, 2024
7eae1e8
Merge branch 'trunk' into update/rename-align-attribute
carolinan Feb 14, 2024
737dda7
Merge branch 'trunk' into update/rename-align-attribute
carolinan Feb 22, 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
2 changes: 1 addition & 1 deletion docs/reference-guides/core-blocks.md
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ Give quoted text visual emphasis. "In quoting others, we cite ourselves." — Ju
- **Name:** core/quote
- **Category:** text
- **Supports:** anchor, color (background, gradients, heading, link, text), typography (fontSize, lineHeight), ~~html~~
- **Attributes:** align, citation, value
- **Attributes:** citation, textAlign, value

## Read More

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ function BlockListItemContent( {
const name = getBlockName( clientId );
const parentName = getBlockName( rootClientId );
const { align } = getBlockAttributes( clientId ) || {};
const { align: parentBlockAlign } =
const { textAlign: parentBlockAlign } =
Copy link
Contributor

Choose a reason for hiding this comment

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

@carolinan, thinking on this more, I realised we may need to co-ordinate releases to ensure centred quote blocks don't break for app users. Would you be able to hold off from merging until I co-ordinate a release plan with the mobile team? I'll update here when we have something in place. Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course.

Copy link
Contributor

Choose a reason for hiding this comment

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

@carolinan, thank you for your patience here! The earliest app release we'd be able to get this into is 24.4, which is set to be released to users on Monday, 18th March. With that in mind, could you merge this after the 17.8 Gutenberg release is cut this coming Wednesday? If this is included in the 17.9 release (set to be released on March 13th), it'd minimise the number of days it's not available for app users.

From my testing, I confirmed that having the change in place on the web but not in the app won't break the site’s content. The only issue I found is that the app's in-editor preview won’t reflect whether a Quote block is centred or right-aligned, which might be confusing. However, this will only be an issue for blocks that are first saved on the web then opened in the app. The in-app alignment buttons will continue to work and correct alignment will still be visible on the site’s front-end.

With the above in mind, we think minimising the number of days that the change is available on one platform but not another is the best option. Let me know if you're okay with delaying merge until after this Wednesday, for the 17.9 release. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it can wait.

Copy link
Contributor

Choose a reason for hiding this comment

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

@carolinan, just to follow up, if you're able to merge this before this coming Thursday, we'll then work to get it into the app's 24.4 release. Thanks again for all your help here!

getBlockAttributes( rootClientId ) || {};

return {
Expand Down
36 changes: 36 additions & 0 deletions packages/block-library/src/paragraph/test/edit.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
initializeEditor,
render,
setupCoreBlocks,
triggerBlockListLayout,
waitFor,
within,
withFakeTimers,
Expand Down Expand Up @@ -210,6 +211,41 @@ describe( 'Paragraph block', () => {
` );
} );

it( 'should inherit parent alignment', async () => {
// Arrange
const screen = await initializeEditor();
await addBlock( screen, 'Quote' );
await triggerBlockListLayout( getBlock( screen, 'Quote' ) );

// Act
const paragraphBlock = getBlock( screen, 'Paragraph' );
fireEvent.press( paragraphBlock );
const paragraphTextInput =
within( paragraphBlock ).getByPlaceholderText( 'Start writing…' );
typeInRichText(
paragraphTextInput,
'A quick brown fox jumps over the lazy dog.'
);
fireEvent.press( screen.getByLabelText( 'Navigate Up' ) );
fireEvent.press( screen.getByLabelText( 'Align text' ) );
fireEvent.press( screen.getByLabelText( 'Align text right' ) );

// Assert
// This not an ideal assertion, as it relies implementation details of the
// component: prop names. However, the only aspect we can assert is the prop
// passed to Aztec, the native module controlling visual alignment. A less
// brittle alternative might be snapshotting, but RNTL does not yet support
// focused snapshots, which means the snapshot would be huge.
// https://github.com/facebook/react/pull/25329
expect(
screen.UNSAFE_queryAllByProps( {
value: '<p>A quick brown fox jumps over the lazy dog.</p>',
placeholder: 'Start writing…',
textAlign: 'right',
} ).length
).toBe( 2 ); // One for Aztec mock, one for the TextInput.
} );

it( 'should preserve alignment when split', async () => {
// Arrange
const screen = await initializeEditor();
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/quote/block.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"default": "",
"__experimentalRole": "content"
},
"align": {
"textAlign": {
"type": "string"
}
},
Expand Down
123 changes: 111 additions & 12 deletions packages/block-library/src/quote/deprecated.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import classnames from 'classnames';
* WordPress dependencies
*/
import { createBlock, parseWithAttributeSchema } from '@wordpress/blocks';
import { RichText, useBlockProps } from '@wordpress/block-editor';
import { InnerBlocks, RichText, useBlockProps } from '@wordpress/block-editor';

export const migrateToQuoteV2 = ( attributes ) => {
const { value, ...restAttributes } = attributes;
Expand All @@ -34,6 +34,87 @@ export const migrateToQuoteV2 = ( attributes ) => {
];
};

const TEXT_ALIGN_OPTIONS = [ 'left', 'right', 'center' ];

// Migrate existing text alignment settings to the renamed attribute.
const migrateTextAlign = ( attributes ) => {
aaronrobertshaw marked this conversation as resolved.
Show resolved Hide resolved
const { align, ...rest } = attributes;
// Check if there are valid alignments stored in the old attribute
// and assign them to the new attribute name.
return TEXT_ALIGN_OPTIONS.includes( align )
? { ...rest, textAlign: align }
: attributes;
};

// Version before the 'align' attribute was replaced with 'textAlign'.
const v4 = {
attributes: {
carolinan marked this conversation as resolved.
Show resolved Hide resolved
value: {
type: 'string',
source: 'html',
selector: 'blockquote',
multiline: 'p',
default: '',
__experimentalRole: 'content',
},
citation: {
type: 'string',
source: 'html',
selector: 'cite',
default: '',
__experimentalRole: 'content',
},
align: {
type: 'string',
},
},
supports: {
anchor: true,
html: false,
__experimentalOnEnter: true,
carolinan marked this conversation as resolved.
Show resolved Hide resolved
__experimentalOnMerge: true,
typography: {
fontSize: true,
lineHeight: true,
__experimentalFontFamily: true,
__experimentalFontWeight: true,
__experimentalFontStyle: true,
__experimentalTextTransform: true,
__experimentalTextDecoration: true,
__experimentalLetterSpacing: true,
__experimentalDefaultControls: {
fontSize: true,
fontAppearance: true,
},
},
color: {
gradients: true,
heading: true,
link: true,
__experimentalDefaultControls: {
background: true,
text: true,
},
},
},
isEligible: ( { align } ) => TEXT_ALIGN_OPTIONS.includes( align ),
save( { attributes } ) {
const { align, citation } = attributes;
const className = classnames( {
[ `has-text-align-${ align }` ]: align,
} );
return (
<blockquote { ...useBlockProps.save( { className } ) }>
<InnerBlocks.Content />
{ ! RichText.isEmpty( citation ) && (
<RichText.Content tagName="cite" value={ citation } />
) }
</blockquote>
);
},
migrate: migrateTextAlign,
};

const v3 = {
attributes: {
value: {
Expand Down Expand Up @@ -87,7 +168,11 @@ const v3 = {
</blockquote>
);
},
migrate: migrateToQuoteV2,
migrate( attributes ) {
const [ v3Attributes, innerBlocks ] = migrateToQuoteV2( attributes );
const v4Attributes = migrateTextAlign( v3Attributes );
return [ v4Attributes, innerBlocks ];
},
};

const v2 = {
Expand All @@ -109,7 +194,11 @@ const v2 = {
type: 'string',
},
},
migrate: migrateToQuoteV2,
migrate( attributes ) {
const [ v2Attributes, innerBlocks ] = migrateToQuoteV2( attributes );
const v4Attributes = migrateTextAlign( v2Attributes );
return [ v4Attributes, innerBlocks ];
aaronrobertshaw marked this conversation as resolved.
Show resolved Hide resolved
},
save( { attributes } ) {
const { align, value, citation } = attributes;

Expand Down Expand Up @@ -151,15 +240,22 @@ const v1 = {
migrate( attributes ) {
if ( attributes.style === 2 ) {
const { style, ...restAttributes } = attributes;
return migrateToQuoteV2( {
...restAttributes,
const [ v1Attributes, innerBlocks ] =
migrateToQuoteV2( restAttributes );
const v4Attributes = migrateTextAlign( v1Attributes );

return {
v4Attributes,
innerBlocks,
className: attributes.className
? attributes.className + ' is-style-large'
: 'is-style-large',
} );
};
aaronrobertshaw marked this conversation as resolved.
Show resolved Hide resolved
}

return migrateToQuoteV2( attributes );
const [ v1Attributes, innerBlocks ] = migrateToQuoteV2( attributes );
const v4Attributes = migrateTextAlign( v1Attributes );
return [ v4Attributes, innerBlocks ];
},

save( { attributes } ) {
Expand Down Expand Up @@ -206,12 +302,15 @@ const v0 = {
migrate( attributes ) {
if ( ! isNaN( parseInt( attributes.style ) ) ) {
const { style, ...restAttributes } = attributes;
return migrateToQuoteV2( {
...restAttributes,
} );
const [ v0Attributes, innerBlocks ] =
migrateToQuoteV2( restAttributes );
const v4Attributes = migrateTextAlign( v0Attributes );
return [ v4Attributes, innerBlocks ];
}

return migrateToQuoteV2( attributes );
const [ v0Attributes, innerBlocks ] = migrateToQuoteV2( attributes );
const v4Attributes = migrateTextAlign( v0Attributes );
return [ v4Attributes, innerBlocks ];
},

save( { attributes } ) {
Expand Down Expand Up @@ -239,4 +338,4 @@ const v0 = {
*
* See block-deprecation.md
*/
export default [ v3, v2, v1, v0 ];
export default [ v4, v3, v2, v1, v0 ];
12 changes: 7 additions & 5 deletions packages/block-library/src/quote/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export default function QuoteEdit( {
className,
style,
} ) {
const { align, citation } = attributes;
const { textAlign, citation } = attributes;

useMigrateOnLoad( attributes, clientId );

Expand All @@ -86,7 +86,7 @@ export default function QuoteEdit( {

const blockProps = useBlockProps( {
className: classNames( className, {
[ `has-text-align-${ align }` ]: align,
[ `has-text-align-${ textAlign }` ]: textAlign,
} ),
...( ! isWebPlatform && { style } ),
} );
Expand All @@ -100,9 +100,9 @@ export default function QuoteEdit( {
<>
<BlockControls group="block">
<AlignmentControl
value={ align }
value={ textAlign }
onChange={ ( nextAlign ) => {
setAttributes( { align: nextAlign } );
setAttributes( { textAlign: nextAlign } );
} }
/>
</BlockControls>
Expand Down Expand Up @@ -132,7 +132,9 @@ export default function QuoteEdit( {
createBlock( getDefaultBlockName() )
)
}
{ ...( ! isWebPlatform ? { textAlign: align } : {} ) }
{ ...( ! isWebPlatform
? { textAlign: attributes.textAlign }
: {} ) }
aaronrobertshaw marked this conversation as resolved.
Show resolved Hide resolved
/>
) }
</BlockQuotation>
Expand Down
4 changes: 2 additions & 2 deletions packages/block-library/src/quote/save.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import classNames from 'classnames';
import { InnerBlocks, RichText, useBlockProps } from '@wordpress/block-editor';

export default function save( { attributes } ) {
const { align, citation } = attributes;
const { textAlign, citation } = attributes;

const className = classNames( {
[ `has-text-align-${ align }` ]: align,
[ `has-text-align-${ textAlign }` ]: textAlign,
} );

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`Quote block transforms to Columns block 1`] = `
"<!-- wp:columns -->
<div class="wp-block-columns"><!-- wp:column {"width":"100%"} -->
<div class="wp-block-column" style="flex-basis:100%"><!-- wp:quote {"align":"left","className":"is-style-large"} -->
<div class="wp-block-column" style="flex-basis:100%"><!-- wp:quote {"textAlign":"left","className":"is-style-large"} -->
<blockquote class="wp-block-quote has-text-align-left is-style-large"><!-- wp:paragraph -->
<p>"This will make running your own blog a viable alternative again."</p>
<!-- /wp:paragraph --><cite>— <a href="https://twitter.com/azumbrunnen_/status/1019347243084800005">Adrian Zumbrunnen</a></cite></blockquote>
Expand All @@ -14,7 +14,7 @@ exports[`Quote block transforms to Columns block 1`] = `

exports[`Quote block transforms to Group block 1`] = `
"<!-- wp:group {"layout":{"type":"constrained"}} -->
<div class="wp-block-group"><!-- wp:quote {"align":"left","className":"is-style-large"} -->
<div class="wp-block-group"><!-- wp:quote {"textAlign":"left","className":"is-style-large"} -->
<blockquote class="wp-block-quote has-text-align-left is-style-large"><!-- wp:paragraph -->
<p>"This will make running your own blog a viable alternative again."</p>
<!-- /wp:paragraph --><cite>— <a href="https://twitter.com/azumbrunnen_/status/1019347243084800005">Adrian Zumbrunnen</a></cite></blockquote>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"isValid": true,
"attributes": {
"citation": "...with a caption",
"align": "right"
"textAlign": "right"
},
"innerBlocks": [
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<!-- wp:quote {"align":"right"} -->
<!-- wp:quote {"textAlign":"right"} -->
<blockquote class="wp-block-quote has-text-align-right"><!-- wp:paragraph -->
<p>Testing deprecated quote block...</p>
<!-- /wp:paragraph --><cite>...with a caption</cite></blockquote>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<!-- wp:quote {"align":"right"} -->
<blockquote class="wp-block-quote has-text-align-right"><!-- wp:paragraph -->
<p>Quote with the align attribute. Example: {"align":"right"}</p>
<!-- /wp:paragraph --></blockquote>
<!-- /wp:quote -->
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[
{
"name": "core/quote",
"isValid": true,
"attributes": {
"value": "",
"citation": "",
"textAlign": "right"
},
"innerBlocks": [
{
"name": "core/paragraph",
"isValid": true,
"attributes": {
"content": "Quote with the align attribute. Example: {\"align\":\"right\"}",
"dropCap": false
},
"innerBlocks": []
}
]
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
[
{
"blockName": "core/quote",
"attrs": {
"align": "right"
},
"innerBlocks": [
{
"blockName": "core/paragraph",
"attrs": {},
"innerBlocks": [],
"innerHTML": "\n<p>Quote with the align attribute. Example: {\"align\":\"right\"}</p>\n",
"innerContent": [
"\n<p>Quote with the align attribute. Example: {\"align\":\"right\"}</p>\n"
]
}
],
"innerHTML": "\n<blockquote class=\"wp-block-quote has-text-align-right\"></blockquote>\n",
"innerContent": [
"\n<blockquote class=\"wp-block-quote has-text-align-right\">",
null,
"</blockquote>\n"
]
}
]
Loading
Loading