Update BlockMover Stories and README #66519

miminari · 2024-10-28T05:13:03Z

What?

Update the BlockMover component's Storybook and README

Why?

I feel Block Editor components's Storybook should also be updated.
We should also have all component's stories, but I started to update the component having stories.
related #22891

How?

Almost Component's stories already have been updated, and use Storybook 7's features.
I changed the Stories based on them, and added and changed the minimum README.

And, I would like to use the automatic citation from the README to the Storybook, but it did not work...

Testing Instructions

npm run storybook:dev and check the BlockMover stories.

Testing Instructions for Keyboard

Screenshots or screencast

github-actions · 2024-10-29T01:36:19Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: miminari <[email protected]>
Co-authored-by: mirka <[email protected]>
Co-authored-by: t-hamano <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

mirka

Thanks for improving our Block Editor stories!

I would like to use the automatic citation from the README to the Storybook, but it did not work

We currently don't have any systems to extract README data into Storybook. I think you're referring to how @wordpress/components stories extract TypeScript/TSDoc data into Storybook 🙂 I don't think we'll be able to automate this part until Block Editor components are annotated with TypeScript.

packages/block-editor/src/components/block-mover/stories/index.story.tsx

mirka · 2024-10-31T09:05:49Z

packages/block-editor/src/components/block-mover/stories/index.story.tsx

+	);
+};
+
+export const Default: StoryFn< typeof BlockMover > = Template.bind( {} );


It might be nice to use CSF 3 format, since we're overhauling anyway.

I just removed the types, is it OK?

I do recommend it in this case, as we can eliminate a lot of unnecessary code.

The main points of this suggested diff being:

The Template component is unnecessary because it's just the plain component, and we only use it once. Plus, the implementation is incorrect because the clientIds prop cannot be overridden by the stories.

The useEffect in the Horizontal story can also be moved to a decorator.

The conditionals that check for blocks.length seem unnecessary because we are assigning the blocks variable statically.

diff --git a/packages/block-editor/src/components/block-mover/stories/index.story.js b/packages/block-editor/src/components/block-mover/stories/index.story.js index 97d8fce96a..241c62c878 100644 --- a/packages/block-editor/src/components/block-mover/stories/index.story.js +++ b/packages/block-editor/src/components/block-mover/stories/index.story.js @@ -31,29 +31,6 @@ const blocks = [ ] ), ]; -function BlockMoverStoryHorizontal() { - const { updateBlockListSettings } = useDispatch( blockEditorStore ); - - useEffect( () => { - /** - * This shouldn't be needed but unfortunately - * the layout orientation is not declarative, we need - * to render the blocks to update the block settings in the state. - */ - updateBlockListSettings( blocks[ 1 ].clientId, { - orientation: 'horizontal', - } ); - }, [] ); - - return ( - <BlockMover - clientIds={ - blocks.length ? [ blocks[ 1 ].innerBlocks[ 1 ].clientId ] : [] - } - /> - ); -} - /** * BlockMover component allows moving blocks inside the editor using up and down buttons. */ @@ -89,55 +66,49 @@ const meta = { }; export default meta; -const Template = ( props ) => { - return ( - <BlockMover - { ...props } - clientIds={ - blocks.length ? [ blocks[ 1 ].innerBlocks[ 1 ].clientId ] : [] - } - /> - ); -}; - -export const Default = Template.bind( {} ); -Default.args = { - clientIds: [ - blocks.length ? [ blocks[ 0 ].innerBlocks[ 1 ].clientId ] : [], - ], +export const Default = { + args: { + clientIds: [ blocks[ 0 ].innerBlocks[ 1 ].clientId ], + }, }; /** * This story shows the block mover with horizontal orientation. * It is necessary to render the blocks to update the block settings in the state. */ -export const Horizontal = ( props ) => { - return <BlockMoverStoryHorizontal { ...props } />; -}; -Horizontal.args = { - clientIds: [ - blocks.length ? [ blocks[ 1 ].innerBlocks[ 1 ].clientId ] : [], +export const Horizontal = { + decorators: [ + ( Story ) => { + const { updateBlockListSettings } = useDispatch( blockEditorStore ); + + useEffect( () => { + /** + * This shouldn't be needed but unfortunately + * the layout orientation is not declarative, we need + * to render the blocks to update the block settings in the state. + */ + updateBlockListSettings( blocks[ 1 ].clientId, { + orientation: 'horizontal', + } ); + }, [] ); + + return <Story />; + }, ], -}; -Horizontal.parameters = { - docs: { canvas: { sourceState: 'hidden' } }, + args: { + clientIds: [ blocks[ 1 ].innerBlocks[ 1 ].clientId ], + }, + parameters: { + docs: { canvas: { sourceState: 'hidden' } }, + }, }; /** * You can hide the drag handle by `hideDragHandle` attribute. */ -export const HideDragHandle = ( props ) => { - return ( - <BlockMover - { ...props } - clientIds={ - blocks.length ? [ blocks[ 1 ].innerBlocks[ 1 ].clientId ] : [] - } - hideDragHandle - /> - ); -}; -HideDragHandle.args = { - ...Default.args, - hideDragHandle: true, +export const HideDragHandle = { + args: { + ...Default.args, + hideDragHandle: true, + }, };

packages/block-editor/src/components/block-mover/stories/index.story.tsx

t-hamano

Thanks for the PR!

Each story is working as expected, but I think one thing we need to address is making the hideDragHandle prop controllable:

672766caf0147a73c5f9aa049172bbf2.mp4

We'll probably need to run updateBlockListSettings in response to changes in the control.

packages/block-editor/src/components/block-mover/README.md

Co-authored-by: Aki Hamano <[email protected]>

….story.tsx Co-authored-by: Lena Morita <[email protected]>

miminari · 2024-11-07T01:09:48Z

@mirka

I would like to use the automatic citation from the README to the Storybook, but it did not work

We currently don't have any systems to extract README data into Storybook. I think you're referring to how @wordpress/components stories extract TypeScript/TSDoc data into Storybook 🙂 I don't think we'll be able to automate this part until Block Editor components are annotated with TypeScript.

I see, thanks for letting me know.
When I tried this, I saw the text rendered in the stories like "Renders a link control. ...", it seems same as the text in the README, so I mistakenly thought I could.

I think I updated all. Could you please review again?

mirka

I suggested some additional cleanup and bug fixes in #66519 (comment), but all the other tweaks look good!

mirka · 2024-11-12T12:19:08Z

packages/block-editor/src/components/block-mover/stories/index.story.js

+	clientIds: [
+		blocks.length ? [ blocks[ 0 ].innerBlocks[ 1 ].clientId ] : [],
+	],


Noting that this would result in a nested array, not a flat one as it should be. (These are also fixed in the suggested diff in #66519 (comment))

mirka · 2024-11-12T12:21:15Z

packages/block-editor/src/components/block-mover/stories/index.story.js

+			clientIds={
+				blocks.length ? [ blocks[ 1 ].innerBlocks[ 1 ].clientId ] : []
+			}
+			hideDragHandle


Noting that these hardcoded props would override any of the prop values passed from args or the Storybook controls.

miminari · 2024-11-14T00:00:01Z

@mirka I appreciate you taking the time to tell me.
I finally got it, I hope.
It turns out at least that I didn't understand 70% of the storybook's functionality...

One point, I was wondering if we should pass the clientIds directly for the Horizontal story because BlockMover can't be rendered horizontally view by passing clientIds via args.
How is it?

t-hamano

Thanks for the update.

Finally, in order for the "Horizontal" story to work properly (to get the hideDragHandle controls to work), it looks like the following changes are necessary:

diff --git a/packages/block-editor/src/components/block-mover/stories/index.story.js b/packages/block-editor/src/components/block-mover/stories/index.story.js
index 5b2fdd4ee6..fd56a4cded 100644
--- a/packages/block-editor/src/components/block-mover/stories/index.story.js
+++ b/packages/block-editor/src/components/block-mover/stories/index.story.js
@@ -80,7 +80,7 @@ export const Default = {
  */
 export const Horizontal = {
        decorators: [
-               () => {
+               ( Story ) => {
                        const { updateBlockListSettings } = useDispatch( blockEditorStore );
                        useEffect( () => {
                                /**
@@ -92,13 +92,12 @@ export const Horizontal = {
                                        orientation: 'horizontal',
                                } );
                        }, [] );
-                       return (
-                               <BlockMover
-                                       clientIds={ [ blocks[ 1 ].innerBlocks[ 1 ].clientId ] }
-                               />
-                       );
+                       return <Story />;
                },
        ],
+       args: {
+               clientIds: [ blocks[ 1 ].innerBlocks[ 1 ].clientId ],
+       },
        parameters: {
                docs: { canvas: { sourceState: 'hidden' } },
        },

packages/block-editor/src/components/block-mover/stories/index.story.js

….story.js Co-authored-by: Aki Hamano <[email protected]>

miminari · 2024-11-14T22:32:03Z

@mirka @t-hamano Sorry, but I missed the point that the implementation of the args was incorrect.
Could you please review again?

t-hamano

LGTM!

431b4dbb370ed576feeea5242ada0dd9.mp4

package.json

miminari added 2 commits October 28, 2024 13:53

Update BlockMover stories

e7707db

Use block settings and provider for Horizontal mode

af4c05a

miminari self-assigned this Oct 29, 2024

Remove unwanted changes

ddbc826

miminari added Storybook Storybook and its stories for components [Type] Code Quality Issues or PRs that relate to code quality [Type] Developer Documentation Documentation for developers and removed [Type] Code Quality Issues or PRs that relate to code quality labels Oct 29, 2024

miminari marked this pull request as ready for review October 29, 2024 01:36

miminari requested a review from ellatrix as a code owner October 29, 2024 01:36

miminari requested review from mirka, adamziel and t-hamano and removed request for ellatrix October 29, 2024 01:36

mirka reviewed Oct 31, 2024

View reviewed changes

t-hamano reviewed Nov 1, 2024

View reviewed changes

packages/block-editor/src/components/block-mover/README.md Outdated Show resolved Hide resolved

packages/block-editor/src/components/block-mover/README.md Show resolved Hide resolved

miminari and others added 5 commits November 7, 2024 08:06

Update packages/block-editor/src/components/block-mover/README.md

ee7f4f1

Co-authored-by: Aki Hamano <[email protected]>

Update packages/block-editor/src/components/block-mover/stories/index…

027e587

….story.tsx Co-authored-by: Lena Morita <[email protected]>

Change .tsx to .js

99b0190

Add decorators, and descriptions

ee98912

Fixes README

a865f22

miminari requested review from mirka and t-hamano November 7, 2024 05:24

mirka reviewed Nov 12, 2024

View reviewed changes

miminari added 3 commits November 14, 2024 08:27

refactor BlockMover stories for improved structure

693b8ee

remove unnecessary lines in package.json

a5dd59d

change Horizontal story to improve rendering

bcd9a92

miminari requested a review from mirka November 14, 2024 00:03

t-hamano reviewed Nov 14, 2024

View reviewed changes

packages/block-editor/src/components/block-mover/stories/index.story.js Outdated Show resolved Hide resolved

miminari and others added 2 commits November 15, 2024 07:17

Update packages/block-editor/src/components/block-mover/stories/index…

66a811d

….story.js Co-authored-by: Aki Hamano <[email protected]>

fix Horizontal decorator

b53d948

miminari requested a review from t-hamano November 14, 2024 22:29

t-hamano approved these changes Nov 15, 2024

View reviewed changes

package.json Outdated Show resolved Hide resolved

remove trash changes from package.json

973b820

t-hamano merged commit 2028fd8 into WordPress:trunk Nov 15, 2024
62 checks passed

github-actions bot added this to the Gutenberg 19.8 milestone Nov 15, 2024

This was referenced Nov 16, 2024

Storybook: Add stories for AlignmentToolbar and AlignmentControl components #67046

Merged

Storybook: Add and Update Block Editor Components stories #67165

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update BlockMover Stories and README #66519

Update BlockMover Stories and README #66519

miminari commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading

mirka left a comment

mirka Oct 31, 2024

miminari Nov 7, 2024

mirka Nov 12, 2024 •

edited

Loading

t-hamano left a comment

miminari commented Nov 7, 2024

mirka left a comment

mirka Nov 12, 2024

mirka Nov 12, 2024

miminari commented Nov 14, 2024

t-hamano left a comment

miminari commented Nov 14, 2024

t-hamano left a comment

Update BlockMover Stories and README #66519

Update BlockMover Stories and README #66519

Conversation

miminari commented Oct 28, 2024 • edited Loading

What?

Why?

How?

Testing Instructions

Testing Instructions for Keyboard

Screenshots or screencast

github-actions bot commented Oct 29, 2024 • edited Loading

mirka left a comment

Choose a reason for hiding this comment

mirka Oct 31, 2024

Choose a reason for hiding this comment

miminari Nov 7, 2024

Choose a reason for hiding this comment

mirka Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

t-hamano left a comment

Choose a reason for hiding this comment

miminari commented Nov 7, 2024

mirka left a comment

Choose a reason for hiding this comment

mirka Nov 12, 2024

Choose a reason for hiding this comment

mirka Nov 12, 2024

Choose a reason for hiding this comment

miminari commented Nov 14, 2024

t-hamano left a comment

Choose a reason for hiding this comment

miminari commented Nov 14, 2024

t-hamano left a comment

Choose a reason for hiding this comment

miminari commented Oct 28, 2024 •

edited

Loading

github-actions bot commented Oct 29, 2024 •

edited

Loading

mirka Nov 12, 2024 •

edited

Loading