-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
refactor: Convert Avatar stories to csf format #7374
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
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.
Looking good. Left some comments. This PR is quite large and addresses several issues that could potentially be split into multiple PRs. To make it easier on the reviewer I would suggest breaking up PRs like this in future. Pulled branch and checked
- for any remaining references to
AvatarVariants
to ensure none were overlooked. - stories, controls and console
app/component-library/components/Avatars/Avatar/Avatar.types.ts
Outdated
Show resolved
Hide resolved
app/component-library/components/Avatars/Avatar/variants/AvatarNetwork/README.md
Show resolved
Hide resolved
app/component-library/components/Avatars/Avatar/variants/AvatarIcon/AvatarIcon.stories.tsx
Show resolved
Hide resolved
app/component-library/components/Avatars/Avatar/variants/AvatarToken/AvatarToken.stories.tsx
Outdated
Show resolved
Hide resolved
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/0d3586f7-8653-4e01-9565-2e91d6ad95b0 |
…to morph/convert-avatars
…to morph/convert-avatars
app/components/Views/Settings/SecuritySettings/__snapshots__/SecuritySettings.test.tsx.snap
Show resolved
Hide resolved
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.
Running into some issues
- Avatar variant favicon is square is that intentional?
- Error on Avatar variant token
Screen.Recording.2023-11-10.at.11.49.32.AM.mov
fixed |
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.
Stories are working but looks like you have some failing tests. This is the video I would like to see as a reviewer to see that you have also checked all the stories. Keep in mind the more stories you add the more that need to be QA'd I would expect these to be checked by the author first. This will also help for faster review
after.mov
…to morph/convert-avatars
…to morph/convert-avatars
Kudos, SonarCloud Quality Gate passed! |
Description
Screenshots/Recordings
after.mov
Related issues
Fixes #???
Pre-merge author checklist
Pre-merge reviewer checklist