- Notifications
You must be signed in to change notification settings - Fork928
test(site): make loading snapshots more predictable#14564
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Abstracts the Spinner component to control the display of the CircularProgress component. This allows us to make it static during Chromatic tests, making loading tests easier to visualize.
| @@ -18,7 +18,7 @@ export const Loader: FC<LoaderProps> = ({ | |||
| data-testid="loader" | |||
| {...attrs} | |||
| > | |||
| <CircularProgress size={size} /> | |||
| <Spinner aria-label="Loading..." size={size} /> | |||
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.
I might be misunderstanding but it seems we have aLoader component wrapping aSpinner component wrapping aCircularProgress component. Is there a reason to have 3 levels nesting here?
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.
Good point! The purpose of the 'Loader' is to act as a container that centrally positions the loading spinner on the page or within a section, with some padding around it. Meanwhile, the 'Spinner' can be utilized in other components, such as loading buttons. Perhaps 'Loader' is not the most suitable name, but I can't think of anything better at the moment.
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.
Ahh, makes sense!Loader works for me :)
Kira-Pilot commentedSep 4, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
One suggestion: we could stick the aria on the actual component so the |
| */ | ||
| if (isChromatic()) { | ||
| props.variant = "determinate"; | ||
| props.value = 75; |
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.
nice
I like the idea! 👍 |
c3f0db3 intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Abstracts the Spinner component to control the display of the CircularProgress component. This allows us to make it static during Chromatic tests, making loading tests easier to visualize.
Before:
After:
