Skip to content

Storybook: Add stories for AlignmentToolbar and AlignmentControl components#67046

Merged
mirka merged 6 commits intoWordPress:trunkfrom
miminari:add/alignment-control-stories
Nov 27, 2024
Merged

Storybook: Add stories for AlignmentToolbar and AlignmentControl components#67046
mirka merged 6 commits intoWordPress:trunkfrom
miminari:add/alignment-control-stories

Conversation

@miminari
Copy link
Member

What?

This PR will add stories for AlignmentToolbar and AlignmentControl components in the Storybook

Why?

Most of Block Editor components don't have stories.
Block Editor components's Storybook should also be added and updated like the Components.
see #66519
related #22891

How?

I have referred to the TextAlignmentControl story.
AlignmentControl and AlignmentToolbar are based on same component, but there are deliberately generated as a component with a different name, so I created in two separate files.

Testing Instructions

npm run storybook:dev and please check the AlignmentControl and AlignmentToolbar stories.

Testing Instructions for Keyboard

Screenshots or screencast

Kapture.2024-11-16.at.13.37.50.mp4

@miminari miminari requested a review from ellatrix as a code owner November 16, 2024 04:42
@miminari miminari self-assigned this Nov 16, 2024
@github-actions
Copy link

github-actions bot commented Nov 16, 2024

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 <mimitips@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>

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

@miminari miminari added Storybook Storybook and its stories for components [Type] Developer Documentation Documentation for developers labels Nov 16, 2024
@miminari miminari requested review from mirka and t-hamano and removed request for ellatrix November 16, 2024 04:43
@miminari miminari marked this pull request as draft November 16, 2024 04:47
@miminari miminari marked this pull request as ready for review November 16, 2024 04:54
@miminari
Copy link
Member Author

@t-hamano Thank you for quick reviewing!
I updated codes. Could you please check again?

Comment on lines +33 to +47
const Template = ( { onChange, ...args } ) => {
const [ value, setValue ] = useState();
return (
<AlignmentToolbar
{ ...args }
onChange={ ( ...changeArgs ) => {
onChange( ...changeArgs );
setValue( ...changeArgs );
} }
value={ value }
/>
);
};

export const Default = Template.bind();
Copy link
Member

Choose a reason for hiding this comment

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

This is fine, but just as a tip for easier authoring and maintenance, it will be a lot simpler to showcase the component in uncontrolled mode if the component supports it. It will literally be a one-liner 😄

Suggested change
const Template = ( { onChange, ...args } ) => {
const [ value, setValue ] = useState();
return (
<AlignmentToolbar
{ ...args }
onChange={ ( ...changeArgs ) => {
onChange( ...changeArgs );
setValue( ...changeArgs );
} }
value={ value }
/>
);
};
export const Default = Template.bind();
export const Default = {};

Copy link
Member Author

Choose a reason for hiding this comment

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

Wow, so easy! And more ideal. Thanks for letting me know!

Copy link
Member Author

Choose a reason for hiding this comment

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

But then we can't reproduce the onChange moving on Storybook, but that's not a problem, right? Or I guess I might miss something...

Copy link
Member

Choose a reason for hiding this comment

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

The onChange should still be observable in the Actions panel as before. Is it not working in your environment? The only reason we need to merge the incoming onChange function is when it's used in controlled mode and you need to call the setValue as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

In my environment, the story of the AlignmentToolbar component does not work properly. The onChange event is fired, but the text alignment is not updated. Does this component not support uncontrolled mode?

d5e018c76f3413012303cbe0189ea76b.mp4

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, my bad! I guess it doesn't support uncontrolled. I'd still recommend using CSF 3 format (instead of the Template.bind()) for new stories though:

export const Default = {
	render: function Template( { onChange, ...args } ) {
		const [ value, setValue ] = useState();
		return (
			<AlignmentToolbar
				{ ...args }
				onChange={ ( ...changeArgs ) => {
					onChange( ...changeArgs );
					setValue( ...changeArgs );
				} }
				value={ value }
			/>
		);
	},
};

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you two! I updated as you wrote.

miminari and others added 2 commits November 22, 2024 09:50
…/aliginment-toolbar.story.js

Co-authored-by: Lena Morita <lena@jaguchi.com>
@miminari miminari requested a review from mirka November 27, 2024 00:07
Copy link
Member

@mirka mirka left a comment

Choose a reason for hiding this comment

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

🚀

@mirka mirka merged commit fedbea3 into WordPress:trunk Nov 27, 2024
@github-actions github-actions bot added this to the Gutenberg 19.9 milestone Nov 27, 2024
@miminari miminari deleted the add/alignment-control-stories branch November 28, 2024 02:31
michalczaplinski pushed a commit that referenced this pull request Dec 5, 2024
…onents (#67046)

* Add stories for AlignmentToolbar and AlignmentControl components

* Remove unneccesery args

* package.json: remove packageManager entry

* Rename the story file and remove unnecessary args from AlignmentControl story

* Update packages/block-editor/src/components/alignment-control/stories/aliginment-toolbar.story.js

Co-authored-by: Lena Morita <lena@jaguchi.com>

* Add render function

---------

Co-authored-by: miminari <mimitips@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Storybook Storybook and its stories for components [Type] Developer Documentation Documentation for developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants