refactor: update more components #3383
Conversation
beyackle
requested review from
a-b-r-o-w-n,
boydc2014,
cwhitten,
srinaath and
tonyanziano
as code owners
…BotFramework-Composer into beyackle/codeCleanup4
…BotFramework-Composer into beyackle/codeCleanup4
| const handlePageSelected = (event: React.FormEvent<HTMLDivElement>, item?: IDropdownOption, index?: number) => { | ||
| setIndex(index || 0); | ||
| onChange(index ? index + 1 : 1); | ||
| if (index == null) return; |
There was a problem hiding this comment.
Double == here. Use === instead.
There was a problem hiding this comment.
I think this is one place I feel comfortable carving out an exception. == null is currently the cleanest way there is to say "is null or undefined", and in TypeScript that's a common thing to say because of how it narrows a ? type into a realized one.
| setIndex(current); | ||
| onChange(current + 1); | ||
| const handleNextClick = () => { | ||
| changePage(index + 1); |
There was a problem hiding this comment.
Consider using useCallback for these two handlers as they have a dependency on index.
There was a problem hiding this comment.
I don't think that would get us much. These handlers only fire if the index changes, so useCallback would just always go off. There's no way to take an action on the paginator that leaves the page unchanged except for the edge case of selecting it from the dropdown (and even them I'm not sure if the component fires its onChange if the same item gets selected as is already selected).
Composer/packages/client/src/components/SkillForm/CreateSkillModal/index.tsx
Outdated
Show resolved
Hide resolved
| @@ -62,12 +61,12 @@ const CreateSkillModal: React.FC<ICreateSkillModalProps> = (props) => { | |||
| const validateForm = (newData: ISkillFormData): ISkillFormDataErrors => { | |||
| const errors: ISkillFormDataErrors = { ...formDataErrors }; | |||
|
|
|||
| if (has(newData, 'manifestUrl')) { | |||
| if (newData.manifestUrl != null) { | |||
There was a problem hiding this comment.
triple equals here and below.
| if (!manifestUrl) { | ||
| export async function validateManifestUrl(projectId: string, manifestUrl: string): Promise<string | undefined> { | ||
| // skip validation if there are other local errors. | ||
| if (manifestUrl == null || manifestUrl === '') { |
* update more components * more small fixes * more small cleanups * fix unit test * Update Onboarding.spec.ts * address PR comments * Update index.tsx * Update Onboarding.spec.ts Co-authored-by: Andy Brown <asbrown002@gmail.com>
Description
This finishes up the Components directory, mostly fixing typos and a small validation bug, and converting Tree to TSX.
Task Item
refs #3307