-
Notifications
You must be signed in to change notification settings - Fork 78
Fix eslint warnings - Final (react-composites) #1600
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "patch", | ||
| "comment": "Fix react useEffect dependencies in composites", | ||
| "packageName": "@internal/react-composites", | ||
| "email": "2684369+JamesBurnside@users.noreply.github.com", | ||
| "dependentChangeType": "patch" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,16 +55,17 @@ export interface MoreDrawerProps extends MoreDrawerDevicesMenuProps { | |
| export const MoreDrawer = (props: MoreDrawerProps): JSX.Element => { | ||
| const drawerMenuItems: DrawerMenuItemProps[] = []; | ||
|
|
||
| const onSelectSpeaker = useCallback( | ||
| const { speakers, onSelectSpeaker } = props; | ||
| const onSpeakerItemClick = useCallback( | ||
| (_ev, itemKey) => { | ||
| const selected = props.speakers?.find((speaker) => speaker.id === itemKey); | ||
| const selected = speakers?.find((speaker) => speaker.id === itemKey); | ||
| if (selected) { | ||
| // This is unsafe - we're only passing in part of the argument to the handler. | ||
| // But this is a known issue in our state. | ||
| props.onSelectSpeaker(selected as AudioDeviceInfo); | ||
| onSelectSpeaker(selected as AudioDeviceInfo); | ||
| } | ||
| }, | ||
| [props.speakers, props.onSelectSpeaker] | ||
| [speakers, onSelectSpeaker] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you need to do this? |
||
| ); | ||
|
|
||
| if (props.speakers && props.speakers.length > 0) { | ||
|
|
@@ -80,23 +81,24 @@ export const MoreDrawer = (props: MoreDrawerProps): JSX.Element => { | |
| : 'MoreDrawerSpeakers' | ||
| }, | ||
| text: speaker.name, | ||
| onItemClick: onSelectSpeaker, | ||
| onItemClick: onSpeakerItemClick, | ||
| secondaryIconProps: isDeviceSelected(speaker, props.selectedSpeaker) ? { iconName: 'Accept' } : undefined | ||
| })), | ||
| secondaryText: props.selectedSpeaker?.name | ||
| }); | ||
| } | ||
|
|
||
| const onSelectMicrophone = useCallback( | ||
| const { microphones, onSelectMicrophone } = props; | ||
| const onMicrophoneItemClick = useCallback( | ||
| (_ev, itemKey) => { | ||
| const selected = props.microphones?.find((mic) => mic.id === itemKey); | ||
| const selected = microphones?.find((mic) => mic.id === itemKey); | ||
| if (selected) { | ||
| // This is unsafe - we're only passing in part of the argument to the handler. | ||
| // But this is a known issue in our state. | ||
| props.onSelectMicrophone(selected as AudioDeviceInfo); | ||
| onSelectMicrophone(selected as AudioDeviceInfo); | ||
| } | ||
| }, | ||
| [props.microphones, props.onSelectMicrophone] | ||
| [microphones, onSelectMicrophone] | ||
| ); | ||
|
|
||
| if (props.microphones && props.microphones.length > 0) { | ||
|
|
@@ -112,7 +114,7 @@ export const MoreDrawer = (props: MoreDrawerProps): JSX.Element => { | |
| : 'MoreDrawerMicrophones' | ||
| }, | ||
| text: mic.name, | ||
| onItemClick: onSelectMicrophone, | ||
| onItemClick: onMicrophoneItemClick, | ||
| secondaryIconProps: isDeviceSelected(mic, props.selectedMicrophone) ? { iconName: 'Accept' } : undefined | ||
| })), | ||
| secondaryText: props.selectedMicrophone?.name | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -101,6 +101,7 @@ export const ChatComposite = (props: ChatCompositeProps): JSX.Element => { | |||
| * @TODO Remove this function and pass the props directly when file-sharing is promoted to stable. | ||||
| * @private | ||||
| */ | ||||
| // eslint-disable-next-line @typescript-eslint/explicit-function-return-type | ||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can use communication-ui-library/packages/react-composites/src/composites/ChatComposite/ChatScreen.tsx Line 77 in e9622a0
|
||||
| const fileSharingOptions = () => { | ||||
| /* @conditional-compile-remove(file-sharing) */ | ||||
| return { | ||||
|
|
||||
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.
nit: I tend to think of
themeas an atomic unit. I'd not expect Contoso to modify parts of the theme so it's OK to addthemeas the dependency (especially if the list of theme fields we reference starts growing)