-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Deprecate 'Post author' block #55352
Open
t-hamano
wants to merge
14
commits into
trunk
Choose a base branch
from
post-author/deprecate
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+253
−11
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
cab6b8e
Deprecate 'Post author' block
t-hamano 72e877b
Merge branch 'trunk' into post-author/deprecate
t-hamano 7390397
Change `inserter` property to `false`
t-hamano 580fc86
Clean empty attributes
t-hamano 491a15e
Incorrect condition when converting Avatar and Biography
t-hamano a8173e3
Use 1em margin as block gap value
t-hamano e29b59f
Merge branch 'trunk' into post-author/deprecate
t-hamano 91c8592
Merge branch 'trunk' into post-author/deprecate
t-hamano 02a9cdd
Merge branch 'trunk' into post-author/deprecate
t-hamano 400ac2d
__next40pxDefaultSize to the migrate button
t-hamano 1a6962b
Merge branch 'trunk' into post-author/deprecate
t-hamano f2cec4a
Resolve conflicts
t-hamano 9875c31
Show migration content via new private slot fill
t-hamano d4c68b2
Merge branch 'trunk' into post-author/deprecate
t-hamano 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
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
27 changes: 27 additions & 0 deletions
27
packages/block-editor/src/components/inspector-controls-last-item-slot-fill/index.js
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 |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { privateApis as componentsPrivateApis } from '@wordpress/components'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { unlock } from '../../lock-unlock'; | ||
| import { | ||
| useBlockEditContext, | ||
| mayDisplayControlsKey, | ||
| } from '../block-edit/context'; | ||
|
|
||
| const { createPrivateSlotFill } = unlock( componentsPrivateApis ); | ||
| const { Fill, Slot } = createPrivateSlotFill( 'InspectorControlsLastItem' ); | ||
|
|
||
| const InspectorControlsLastItem = ( props ) => { | ||
| const context = useBlockEditContext(); | ||
| if ( ! context[ mayDisplayControlsKey ] ) { | ||
| return null; | ||
| } | ||
| return <Fill { ...props } />; | ||
| }; | ||
| InspectorControlsLastItem.Slot = ( props ) => <Slot { ...props } />; | ||
|
|
||
| export default InspectorControlsLastItem; |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { migrateToRecommendedBlocks } from './utils'; | ||
|
|
||
| const transforms = { | ||
| to: [ | ||
| { | ||
| type: 'block', | ||
| blocks: [ 'core/group' ], | ||
| transform: ( attributes ) => | ||
| migrateToRecommendedBlocks( attributes ), | ||
| }, | ||
| ], | ||
| }; | ||
|
|
||
| export default transforms; |
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 |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { createBlock } from '@wordpress/blocks'; | ||
| import { __ } from '@wordpress/i18n'; | ||
| import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; | ||
|
|
||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { unlock } from '../lock-unlock'; | ||
|
|
||
| const { cleanEmptyObject } = unlock( blockEditorPrivateApis ); | ||
|
|
||
| /** | ||
| * Generate Author-related blocks based on block attributes. | ||
| * | ||
| * @param {Object} attributes Block's attributes. | ||
| * | ||
| * @return {Object} Generated block. | ||
| */ | ||
| export function migrateToRecommendedBlocks( attributes ) { | ||
| const { | ||
| avatarSize, | ||
| byline, | ||
| showAvatar, | ||
| showBio, | ||
| isLink, | ||
| linkTarget, | ||
| textAlign, | ||
| style, | ||
| ...restAttributes | ||
| } = attributes; | ||
|
|
||
| return createBlock( | ||
| 'core/group', | ||
| { | ||
| ...restAttributes, | ||
| style: cleanEmptyObject( { | ||
| ...style, | ||
| spacing: { | ||
| blockGap: '1em', | ||
| }, | ||
| color: { | ||
| ...style?.color, | ||
| // Duotone must be applied to the avatar block. | ||
| duotone: undefined, | ||
| }, | ||
| } ), | ||
| layout: { | ||
| type: 'flex', | ||
| flexWrap: 'nowrap', | ||
| verticalAlignment: 'top', | ||
| }, | ||
| }, | ||
| [ | ||
| showAvatar && | ||
| createBlock( 'core/avatar', { | ||
| size: avatarSize, | ||
| style: cleanEmptyObject( { | ||
| border: { | ||
| radius: '0px', | ||
| }, | ||
| color: { | ||
| duotone: style?.color?.duotone, | ||
| }, | ||
| } ), | ||
| } ), | ||
| createBlock( | ||
| 'core/group', | ||
| { | ||
| style: { | ||
| layout: { | ||
| selfStretch: 'fill', | ||
| flexSize: null, | ||
| }, | ||
| }, | ||
| layout: { | ||
| type: 'flex', | ||
| orientation: 'vertical', | ||
| justifyContent: textAlign, | ||
| }, | ||
| }, | ||
| [ | ||
| createBlock( 'core/paragraph', { | ||
| content: byline, | ||
| placeholder: __( 'Write byline…' ), | ||
| style: { | ||
| typography: { | ||
| fontSize: '0.5em', | ||
| }, | ||
| }, | ||
| } ), | ||
| createBlock( 'core/post-author-name', { | ||
| isLink, | ||
| linkTarget, | ||
| style: { | ||
| typography: { | ||
| fontSize: '1em', | ||
| }, | ||
| }, | ||
| } ), | ||
| showBio && | ||
| createBlock( 'core/post-author-biography', { | ||
| style: { | ||
| typography: { | ||
| fontSize: '0.7em', | ||
| }, | ||
| }, | ||
| } ), | ||
| ].filter( Boolean ) | ||
| ), | ||
| ].filter( Boolean ) | ||
| ); | ||
| } | ||
Oops, something went wrong.
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.
The layout and spacing changes are a bit unexpected when migrating the block. Can we apply some sensible defaults to match the previous spacing?
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.
The Post Author block also has a Flex layout, but seems to have a 1em right margin between the avatar and the content. Therefore, in a8173e3, this is used as the
styles.spacing.blockGapvalue.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.
Thanks for iterating on this. The block gap spacing is only one aspect of the issue. In the video I shared you'll notice that the overall width of the block change. That is unexpected and could further break themes.
Before
After
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.
Regarding the width of the block, I think the Post Author block does not have
box-sizing:border-box, so the width change will occur if left and right padding is applied.This spacing support for this block was added in #35963, but this PR was before #43243 that recommends adding
box-sizing:border-boxat the same time as supporting spacing controls.Originally, I think the Post Author block should also have
box-sizing:border-box, but what approach do you think is best to prevent breaking changes as much as possible? Personally, I think that increasing the width of the Post Author block by padding is not originally intended, and that changing the width may be acceptable for migration purposes.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.
Update: Post Author block now supports borders, as of #64599. At the same time,
box-sizing: border-boxwas added, so there should be no width change when migrating.