-
Notifications
You must be signed in to change notification settings - Fork 71
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
[Storybook v8] Add file for Participant List #4818
Conversation
CallWithChat bundle size is increased❗.
|
Calling bundle size is increased❗.
|
Chat bundle size is increased❗.
|
@azure/communication-react jest test coverage for stable.
|
@azure/communication-react jest test coverage for beta.
|
@@ -0,0 +1,46 @@ | |||
import { ParticipantList } from '@azure/communication-react'; | |||
import { Title, Heading, Description, Canvas, Props, Meta } from '@storybook/addon-docs'; |
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.
There are a couple imports here that aren't used: Title, Heading, Description, Props
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.
aren't we using @storybook/blocks
for these imports and not @storybook/addon-docs
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.
Both blocks
and addon-docs
work
|
||
// eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
const onParticipantRemove = (_userId: string): void => { | ||
// Do something when remove a participant from list |
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.
Suggestion: In the image overlay we display an alert pop up, maybe we can do something like that so no need to force the eslint disable
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.
Just a few nitpick suggestions
@@ -0,0 +1,46 @@ | |||
import { ParticipantList } from '@azure/communication-react'; | |||
import { Title, Heading, Description, Canvas, Props, Meta } from '@storybook/addon-docs'; |
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.
aren't we using @storybook/blocks
for these imports and not @storybook/addon-docs
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.
storyshots are deprecated, we should be removing this file
Pull Request is not mergeable
Failed to pass the UI Test. If this PR is for UI change and the error is snapshot mismatch, please add "update_snapshots" label to the PR for updating the snapshot. |
What
Why
How Tested
Process & policy checklist
Is this a breaking change?