From 715abefe3454205a81853ce75bc11622539d36d9 Mon Sep 17 00:00:00 2001 From: Krishna Gupta Date: Thu, 28 Mar 2024 07:48:17 +0530 Subject: [PATCH 1/4] fix: Focus not removed when selection list changes. Signed-off-by: Krishna Gupta --- .../SelectionList/BaseSelectionList.tsx | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 32cd89854cf..a0071c52fa1 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -393,18 +393,29 @@ function BaseSelectionList( ); const prevTextInputValue = usePrevious(textInputValue); + const prevSelectedOptionsLength = usePrevious(flattenedSections.selectedOptions.length); + useEffect(() => { // Avoid changing focus if the textInputValue remains unchanged. - if (prevTextInputValue === textInputValue || flattenedSections.allOptions.length === 0) { + if ((prevTextInputValue === textInputValue && flattenedSections.selectedOptions.length === prevSelectedOptionsLength) || flattenedSections.allOptions.length === 0) { return; } - // Remove the focus if the search input is empty else focus on the first non disabled item - const newSelectedIndex = textInputValue === '' ? -1 : 0; + // Remove the focus if the search input is empty or selected options length is changed else focus on the first non disabled item + const newSelectedIndex = textInputValue === '' || flattenedSections.selectedOptions.length !== prevSelectedOptionsLength ? -1 : 0; + // reseting the currrent page to 1 when the user types something setCurrentPage(1); updateAndScrollToFocusedIndex(newSelectedIndex); - }, [canSelectMultiple, flattenedSections.allOptions.length, prevTextInputValue, textInputValue, updateAndScrollToFocusedIndex]); + }, [ + canSelectMultiple, + flattenedSections.allOptions.length, + flattenedSections.selectedOptions.length, + prevTextInputValue, + textInputValue, + updateAndScrollToFocusedIndex, + prevSelectedOptionsLength, + ]); useEffect( () => () => { From b91c08943535c0cca685f5564c3dbb9abc0fda57 Mon Sep 17 00:00:00 2001 From: Krishna Gupta Date: Fri, 29 Mar 2024 08:46:55 +0530 Subject: [PATCH 2/4] fix: perf tests. Signed-off-by: Krishna Gupta --- src/components/SelectionList/BaseSelectionList.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index a0071c52fa1..2fc63f3cd10 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -393,15 +393,15 @@ function BaseSelectionList( ); const prevTextInputValue = usePrevious(textInputValue); - const prevSelectedOptionsLength = usePrevious(flattenedSections.selectedOptions.length); + const prevSelectedOptions = usePrevious(flattenedSections.selectedOptions); useEffect(() => { // Avoid changing focus if the textInputValue remains unchanged. - if ((prevTextInputValue === textInputValue && flattenedSections.selectedOptions.length === prevSelectedOptionsLength) || flattenedSections.allOptions.length === 0) { + if ((prevTextInputValue === textInputValue && flattenedSections.selectedOptions.length === prevSelectedOptions.length) || flattenedSections.allOptions.length === 0) { return; } // Remove the focus if the search input is empty or selected options length is changed else focus on the first non disabled item - const newSelectedIndex = textInputValue === '' || flattenedSections.selectedOptions.length !== prevSelectedOptionsLength ? -1 : 0; + const newSelectedIndex = textInputValue === '' || flattenedSections.selectedOptions.length !== prevSelectedOptions.length ? -1 : 0; // reseting the currrent page to 1 when the user types something setCurrentPage(1); @@ -414,7 +414,7 @@ function BaseSelectionList( prevTextInputValue, textInputValue, updateAndScrollToFocusedIndex, - prevSelectedOptionsLength, + prevSelectedOptions, ]); useEffect( From 2a3bc3f1f7fd800cdd2e8117b77bd556024caf4b Mon Sep 17 00:00:00 2001 From: Krishna Gupta Date: Fri, 5 Apr 2024 11:51:21 +0530 Subject: [PATCH 3/4] fix: Reassure Performance Tests. Signed-off-by: Krishna Gupta --- src/components/SelectionList/BaseSelectionList.tsx | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index b9156f1d098..6530316da8d 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -409,15 +409,8 @@ function BaseSelectionList( setCurrentPage(1); updateAndScrollToFocusedIndex(newSelectedIndex); - }, [ - canSelectMultiple, - flattenedSections.allOptions.length, - flattenedSections.selectedOptions.length, - prevTextInputValue, - textInputValue, - updateAndScrollToFocusedIndex, - prevSelectedOptions, - ]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [canSelectMultiple, flattenedSections.allOptions.length, flattenedSections.selectedOptions.length, prevTextInputValue, textInputValue, updateAndScrollToFocusedIndex]); useEffect( () => () => { From 1b876b1e3b51c08916e93f075de05e1ebe7f7037 Mon Sep 17 00:00:00 2001 From: Krishna Gupta Date: Fri, 5 Apr 2024 12:13:28 +0530 Subject: [PATCH 4/4] add all deps. Signed-off-by: Krishna Gupta --- .../SelectionList/BaseSelectionList.tsx | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/components/SelectionList/BaseSelectionList.tsx b/src/components/SelectionList/BaseSelectionList.tsx index 6530316da8d..ce987b32477 100644 --- a/src/components/SelectionList/BaseSelectionList.tsx +++ b/src/components/SelectionList/BaseSelectionList.tsx @@ -395,22 +395,29 @@ function BaseSelectionList( ); const prevTextInputValue = usePrevious(textInputValue); - const prevSelectedOptions = usePrevious(flattenedSections.selectedOptions); + const prevSelectedOptionsLength = usePrevious(flattenedSections.selectedOptions.length); useEffect(() => { // Avoid changing focus if the textInputValue remains unchanged. - if ((prevTextInputValue === textInputValue && flattenedSections.selectedOptions.length === prevSelectedOptions.length) || flattenedSections.allOptions.length === 0) { + if ((prevTextInputValue === textInputValue && flattenedSections.selectedOptions.length === prevSelectedOptionsLength) || flattenedSections.allOptions.length === 0) { return; } // Remove the focus if the search input is empty or selected options length is changed else focus on the first non disabled item - const newSelectedIndex = textInputValue === '' || flattenedSections.selectedOptions.length !== prevSelectedOptions.length ? -1 : 0; + const newSelectedIndex = textInputValue === '' || flattenedSections.selectedOptions.length !== prevSelectedOptionsLength ? -1 : 0; // reseting the currrent page to 1 when the user types something setCurrentPage(1); updateAndScrollToFocusedIndex(newSelectedIndex); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [canSelectMultiple, flattenedSections.allOptions.length, flattenedSections.selectedOptions.length, prevTextInputValue, textInputValue, updateAndScrollToFocusedIndex]); + }, [ + canSelectMultiple, + flattenedSections.allOptions.length, + flattenedSections.selectedOptions.length, + prevTextInputValue, + textInputValue, + updateAndScrollToFocusedIndex, + prevSelectedOptionsLength, + ]); useEffect( () => () => {