Skip to content
This repository was archived by the owner on May 1, 2025. It is now read-only.

Create bouncingDots Loader Component#391

Merged
bereket-WMDE merged 5 commits intomasterfrom
create_bouncingDots_loader
Mar 11, 2021
Merged

Create bouncingDots Loader Component#391
bereket-WMDE merged 5 commits intomasterfrom
create_bouncingDots_loader

Conversation

@bereket-WMDE
Copy link
Copy Markdown
Contributor

Bug: T267010

  • creates the bouncing dot loader component
  • creates story documentation

@github-actions
Copy link
Copy Markdown


basic.argTypes = {
position: {
control: {
Copy link
Copy Markdown
Contributor

@sai-san sai-san Mar 11, 2021

Choose a reason for hiding this comment

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

we can remove the controls, since both sizes are visible within the same story

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This configuration can then be replaced with something like there:

all.parameters = {
controls: { hideNoControlsWarning: true },
};

See also https://storybook.js.org/docs/react/essentials/controls#hide-nocontrols-warning
(and the entire page might be of interest in general)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@SaiSan-WMDE thank you for the update
@micgro42 thanks for the resource

title: '/Loader/BouncingDots',
};

export function basic( args: { content: string; underlined: boolean } ): Component {
Copy link
Copy Markdown
Contributor

@sai-san sai-san Mar 11, 2021

Choose a reason for hiding this comment

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

I believe this story can be called "All", since it contains all size variants

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

And since this is supposed to not contain controls, we con drop the args parameter and every place where it is used.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed unnecessary args parameter

Comment thread vue-components/stories/BouncingDots.stories.ts Outdated
@@ -0,0 +1 @@
export const bouncingDotsSizes = [ 'small', 'medium' ];
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It might be that things get confused by having the same filename that differs only by extension and capitalization. Better to rename this file to bouncingDotsProps.ts or something like that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yea makes sense to change the name

bereket-WMDE and others added 2 commits March 11, 2021 10:54
Co-authored-by: Sai_San <sarai.sanchez@wikimedia.de>
- removed unnecessary control from the stroy
@bereket-WMDE bereket-WMDE force-pushed the create_bouncingDots_loader branch from a8b8cae to 33a82ca Compare March 11, 2021 10:43
@sai-san sai-san self-requested a review March 11, 2021 16:05
Copy link
Copy Markdown
Contributor

@sai-san sai-san left a comment

Choose a reason for hiding this comment

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

LGTM!

giphy

@bereket-WMDE bereket-WMDE merged commit 80c8ddc into master Mar 11, 2021
@bereket-WMDE bereket-WMDE deleted the create_bouncingDots_loader branch March 11, 2021 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants