New Controls SelectTeamPicker/SelectTeamChannelPicker

[joaojmendes]

Q	A

| New sample? | [ X]

What's in this Pull Request?

New Controls:

SelectTeamPicker and SelectTeamChannelPicker

[AJIXuMuK]

Thank you @joaojmendes for the great addition to the library!
While SPFx v1.12 is deprecated, we have some time to make some changes to the PR.
I reviewed the code partially and left some comments.
Most of them are related to

• importing items from office-ui-fabric-react instead of office-ui-fabric-react/lib/...
• using useCallback with empty array of dependencies which could lead to issues.

Please, if you have some spare time, update the code based on these comments.

Thanks!

[joaojmendes]

Hi Alex, Thank you very much, I will do the changes soon as possible. Thanks! João From: Alex Terentiev ***@***.***> Date: Wednesday, 24 March 2021 at 15:50 To: pnp/sp-dev-fx-controls-react ***@***.***> Cc: João Mendes ***@***.***>, Mention ***@***.***> Subject: Re: [pnp/sp-dev-fx-controls-react] New Controls SelectTeamPicker/SelectTeamChannelPicker (#846) @AJIXuMuK requested changes on this pull request. Thank you @joaojmendes<https://github.com/joaojmendes> for the great addition to the library! While SPFx v1.12 is deprecated, we have some time to make some changes to the PR. I reviewed the code partially and left some comments. Most of them are related to * importing items from office-ui-fabric-react instead of office-ui-fabric-react/lib/... * using useCallback with empty array of dependencies which could lead to issues. Please, if you have some spare time, update the code based on these comments. Thanks!

________________________________ In src/controls/SelectTeamChannelPicker/constants.ts<#846 (comment)>:

@@ -0,0 +1 @@

+export const TEAMS_SVG_LOGO = "https://static2.sharepointonline.com/files/fabric-cdn-prod_20200430.002/assets/brand-icons/product/svg/teams_48x1.svg"; I think to be on a save side of things we need to use https://static2.sharepointonline.com/files/fabric/assets/brand-icons/product/svg/teams_48x1.svg as icon URL. See this documentation<https://developer.microsoft.com/en-us/fluentui#/styles/web/office-brand-icons>

________________________________ In src/SelectTeamChannelPicker.ts<#846 (comment)>:

@@ -0,0 +1 @@

+export * from "./controls/SelectTeamChannelPicker"; Would it be better to rename the controls to TeamChannelPicker? In that case the name will be aligned with other pickers.

________________________________ In src/SelectTeamPicker.ts<#846 (comment)>:

@@ -0,0 +1 @@

+export * from "./controls/SelectTeamPicker"; Would it be better to rename the controls to TeamPicker? In that case the name will be aligned with other pickers.

________________________________ In src/controls/SelectTeamChannelPicker/ISelectTeamChannelPickerProps.ts<#846 (comment)>:

@@ -0,0 +1,13 @@

+import { WebPartContext } from ***@***.***/sp-webpart-base"; +import { ExtensionContext } from ***@***.***/sp-extension-base"; +import { IBasePickerStyles, IPickerItemProps, ISuggestionItemProps, ITag } from "office-ui-fabric-react"; +export interface ISelectTeamChannelPickerProps { + teamId:string | number; + appcontext: WebPartContext | ExtensionContext; We can use BaseComponentContext instead of WebPartContext | ExtensionContext.

________________________________ In src/controls/SelectTeamChannelPicker/ISelectTeamChannelPickerProps.ts<#846 (comment)>:

@@ -0,0 +1,13 @@

+import { WebPartContext } from ***@***.***/sp-webpart-base"; +import { ExtensionContext } from ***@***.***/sp-extension-base"; +import { IBasePickerStyles, IPickerItemProps, ISuggestionItemProps, ITag } from "office-ui-fabric-react"; Please, use specific imports, otherwise you're referencing the whole OUIFR and increase the size of our lib.

________________________________ In src/controls/SelectTeamChannelPicker/ISelectTeamChannelPickerState.ts<#846 (comment)>:

@@ -0,0 +1,5 @@

+import { ITag } from "office-ui-fabric-react"; Please, use specific imports, otherwise you're referencing the whole OUIFR and increase the size of our lib.

________________________________ In src/controls/SelectTeamChannelPicker/SelectTeamChannelPicker.tsx<#846 (comment)>:

+ IBasePickerSuggestionsProps,

+ IPickerItemProps, + ISuggestionItemProps, + ISuggestionsItem, +} from "office-ui-fabric-react/lib/Pickers"; +import { useTeams } from "../../hooks"; +import { ITeamChannel } from "./../../common/model/ITeamChannel"; +import { ISelectTeamChannelPickerProps } from "./ISelectTeamChannelPickerProps"; +import { + IconButton, + Stack, + Text, + FontIcon, + Label, +} from "office-ui-fabric-react"; +import { find, pullAllBy } from "lodash"; Please, use specific imports for lodash

________________________________ In src/controls/SelectTeamChannelPicker/SelectTeamChannelPicker.tsx<#846 (comment)>:

+ const checkExists = find(teamsChannelList, { key: teamChannel.id });

+ if (checkExists) continue; + tags.push({ + key: teamChannel.id, + name: `${teamChannel.displayName},${teamChannel.membershipType},${ + teamChannel.isFavoriteByDefault ?? "false" + }`, + }); + } + } + } catch (error) { + console.log(error); + } + return tags; + }, + [] useCallback with empty dependency array may lead to issues. For example, if you use some state variables in the callback, they might always have undefined value as the callback was defined before any value has been set to the var. For this one props or props.teamId potentially can be undefined or empty.

________________________________ In src/controls/SelectTeamChannelPicker/SelectTeamChannelPicker.tsx<#846 (comment)>:

+ [itemProps.item]

+ ); + dispatch({ + type: "UPDATE_SELECTEITEM", + payload: _newSelectedTeamsChannels, + }); + props.onSelectedChannels(_newSelectedTeamsChannels); + }} + /> + </Stack> + ); + } else { + return null; + } + }, + [state.selectedTeamsChannels] you are using props.onSelectedChannels in the callback, but it is not listed in the dependencies

________________________________ In src/controls/SelectTeamChannelPicker/SelectTeamChannelPickerStyles.ts<#846 (comment)>:

@@ -0,0 +1,213 @@

+import { + IBasePickerStyles, + IButtonStyles, + IStackStyles, + ITextStyles, + mergeStyles, + mergeStyleSets, +} from "office-ui-fabric-react"; Please, use, specific imports from OUIFR

________________________________ In src/controls/SelectTeamPicker/ISelectTeamPickerProps.ts<#846 (comment)>:

@@ -0,0 +1,11 @@

+import { WebPartContext } from ***@***.***/sp-webpart-base"; +import { ExtensionContext } from ***@***.***/sp-extension-base"; +import { IBasePickerStyles, IPickerItemProps, ISuggestionItemProps, ITag } from "office-ui-fabric-react"; +export interface ISelectTeamPickerProps { + appcontext: WebPartContext | ExtensionContext; We can use BaseComponentContext instead

________________________________ In src/controls/SelectTeamPicker/SelectTeamPicker.tsx<#846 (comment)>:

+ IBasePicker,

+ ITag, + IBasePickerSuggestionsProps, + IPickerItemProps, + ISuggestionItemProps, +} from "office-ui-fabric-react/lib/Pickers"; +import { useTeams } from "../../hooks"; +import { ITeam } from "./../../common/model/ITeam"; +import { ISelectTeamPickerProps } from "./ISelectTeamPickerProps"; +import { + IconButton, + Stack, + Text, + ImageIcon, + Label, +} from "office-ui-fabric-react"; Please use specific imports from OUIFR.

________________________________ In src/controls/SelectTeamPicker/SelectTeamPicker.tsx<#846 (comment)>:

+ ITag,

+ IBasePickerSuggestionsProps, + IPickerItemProps, + ISuggestionItemProps, +} from "office-ui-fabric-react/lib/Pickers"; +import { useTeams } from "../../hooks"; +import { ITeam } from "./../../common/model/ITeam"; +import { ISelectTeamPickerProps } from "./ISelectTeamPickerProps"; +import { + IconButton, + Stack, + Text, + ImageIcon, + Label, +} from "office-ui-fabric-react"; +import { find, pullAllBy } from "lodash"; Please use specific imports from lodash — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#846 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AC7YHVMFOU2RXVCI5MEDN5LTFIC3NANCNFSM4ZV4ZDFQ>.

[AJIXuMuK]

Thank you @joaojmendes for quick fixes!

There are few minor things though. Could you please look into it?

Thank you so much!

[joaojmendes]

Hi @AJIXuMuK thank you very much for your support :) you are awesome

[AJIXuMuK]

Thank you @joaojmendes for the changes

[AJIXuMuK]

@joaojmendes - it has been merged and will be included in the next v3 release.
Thank you again!