[C+ Checklist Needs Completion] [$250] The focused background isn't updated when the selected option is updated from BE side

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 9.0.75-6
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @DylanDylann
Slack conversation (hyperlinked to channel name): expensify_bugs

Action Performed:

1. In the device 1, go to Settings> Preferences > Priority mode
2. In the device 2, go to Settings> Preferences > Priority mode
3. In the device 2, click on another option

Expected Result:

The selected option is updated in the device 1 but the focus background isn't updated

Actual Result:

The focused background isn't updated in the device 1

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Screen.Recording.2024-12-12.at.11.29.54.mov

Recording.844.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021867633562881780913
• Upwork Job ID: 1867633562881780913
• Last Price Increase: 2024-12-13
• Automatic offers:
• DylanDylann | Reviewer | 105417829
• daledah | Contributor | 105417830

Issue Owner

Current Issue Owner: @michaelkwardrop

[melvin-bot]

Triggered auto assignment to @michaelkwardrop (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[nyomanjyotisa]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The focused background isn't updated when the selected option is updated from BE side

What is the root cause of that problem?

The focus is not updated when the selected priority mode changes, causing the UI to stay focused on the old selection.

What changes do you think we should make in order to solve the problem?

We should manage the focus using a state variable (focusedKey) that gets updated when the priorityMode changes. This ensures the UI focus stays in sync with the selected priority mode.

    const [focusedKey, setFocusedKey] = useState(priorityMode);

    useEffect(() => {
        setFocusedKey(priorityMode);
    }, [priorityMode]);

And change the SelectionList to this

            <SelectionList
                sections={[{data: priorityModes}]}
                ListItem={RadioListItem}
                onSelectRow={(mode) => {
                    updateMode(mode);
                    setFocusedKey(mode.keyForList);
                }}
                shouldSingleExecuteRowSelect
                initiallyFocusedOptionKey={focusedKey}
                key={focusedKey}
            />

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

POC

New-Expensify.mp4

[linhvovan29546]

Edited by proposal-police: This proposal was edited at 2024-12-13 02:47:13 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

The focused background isn't updated when the selected option is updated from BE side

What is the root cause of that problem?

In BaseSelectionList, the getItemBackgroundColorStyle function is used to get the background color for an item. This function only applies a background color if isFocused is true. When updates are received from the backend, only the isSelected state of priorityModes is updated, but isFocused is not. As a result, the focused background does not change.

App/src/components/SelectionList/BaseListItem.tsx

Line 113 in 10bfe9a

isFocused && StyleUtils.getItemBackgroundColorStyle(!!item.isSelected, !!isFocused, !!item.isDisabled, theme.activeComponentBG, theme.hoverComponentBG),

What changes do you think we should make in order to solve the problem?

Out dated

In PreferencesPage, we should use a useEffect with priorityModes as a dependency to determine the focusedIndex and then call ref.current.setFocusedIndex(focusedIndex).

useEffect(() => {
        const handleFocus = () => {
            const focusedIndex = priorityModes.findIndex((mode) => mode.isSelected)
            refSelectionList.current?.setFocusedIndex(focusedIndex)
        }
        handleFocus()
    }, [priorityModes])
.....
            <SelectionList
                ref={refSectionList}
                //    other props
             />

We also need to export the setFocusedIndex method from BaseSelectionList using useImperativeHandle to enable this functionality.

App/src/components/SelectionList/BaseSelectionList.tsx

Lines 735 to 741 in 10bfe9a

	useImperativeHandle(ref, () => ({scrollAndHighlightItem, clearInputAfterSelect, updateAndScrollToFocusedIndex, updateExternalTextInputFocus, scrollToIndex}), [
	scrollAndHighlightItem,
	clearInputAfterSelect,
	updateAndScrollToFocusedIndex,
	updateExternalTextInputFocus,
	scrollToIndex,
	]);

New proposal:

In the BaseSelectionList add a new prop shouldAutoUpdateFocusedIndex, with a default value of false. This prop will be set to true only in the specific places where the focused index needs to be updated automatically, in order to avoid regression.
Use useEffect with flattenedSections as a dependency to update focusedIndex

    const isRenderRef = useRef(false)

    useEffect(() => {
        if (!shouldAutoUpdateFocusedIndex) return
        if (!isRenderRef.current) {
            isRenderRef.true
            return
        }
        const handleUpdateFocusedIndex = () => {
            const focusedIndex = flattenedSections.allOptions.findIndex((section) => section.isSelected)
            setFocusedIndex(focusedIndex)
        }
        handleUpdateFocusedIndex()
    }, [shouldAutoUpdateFocusedIndex, flattenedSections.allOptions])

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

Edited at 2024-12-14 04:46:13 UTC.
I don't see any test files for this, so I think we need to implement UI tests for the BaseSelectionList component, including the following actions: render the BaseSelectionList with the correct selected status, handling item press, and rerender the BaseSelectionList with the updated selected status.

N/A

What alternative solutions did you explore? (Optional)

N/A

[daledah]

Edited by proposal-police: This proposal was edited at 2024-12-19 16:20:50 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

The focused background isn't updated in the device 1

What is the root cause of that problem?

In BaseSelectionList, we don't have logics to change focusedIndex when there's update from BE.

This Bug also appears on similar pages: Language and Theme settings.

What changes do you think we should make in order to solve the problem?

Add an useEffect to change focusedIndex when list data is updated to after here:

App/src/components/SelectionList/BaseSelectionList.tsx

Line 337 in 10bfe9a

}, [onChangeText]);

    useEffect(() => {
        const selectedItemIndex = flattenedSections.allOptions.findIndex((option) => option.isSelected);
        if (selectedItemIndex === -1 || selectedItemIndex === focusedIndex) {
            return;
        }
        setFocusedIndex(selectedItemIndex);
    }, [flattenedSections]);

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

For this specific issue, we can introduce these test cases for BaseSelectionList:

1. Item press

    it('should handle item press correctly', () => {
        render(<RenderBaseSelectionList sections={[{data: SECTIONS}]} />);
        fireEvent.press(screen.getByTestId('base-list-item-2'));
        expect(mockOnSelectRow).toHaveBeenCalledWith({
            ...SECTIONS.at(1),
            shouldAnimateInHighlight: false,
        });
    });

2. Changes to data from BE

    it('should update focused item when sections are updated from BE', () => {
        const updatedSections = SECTIONS.map((section) => ({
            ...section,
            isSelected: section.keyForList === '2',
        }));
        const {rerender} = render(<RenderBaseSelectionList sections={[{data: SECTIONS}]} />);
        fireEvent.press(screen.getByTestId('base-list-item-1'));
        rerender(<RenderBaseSelectionList sections={[{data: updatedSections}]} />);
        expect(screen.getByTestId('base-list-item-2')).toHaveAccessibilityValue({
            selected: true,
        });
    });

3. Optionally, we can test for keyboard control as well, but this test will require to install an additional package @testing-library/user-event:

    it('should handle keyboard navigation correctly', async () => {
        render(<RenderBaseSelectionList sections={[{data: SECTIONS}]} />);
        await userEvent.keyboard('{arrowdown}');
        expect(screen.getByTestId('base-list-item-2', {})).toHaveAccessibilityValue({
            selected: true,
        });
    });

What alternative solutions did you explore? (Optional)

We can pass key to SelectionList in PriorityModePage to force re-render when priority mode is changed, however this can cause unwanted extra render count and make the UI briefly flashes but still noticeable.

[DylanDylann]

hey contributors, please make sure that your proposal also fix this problem

[DylanDylann]

@michaelkwardrop Let me take over this bug because it came from https://expensify.slack.com/archives/C05LX9D6E07/p1733996291225739?thread_ts=1730403194.757359&cid=C05LX9D6E07

cc @mountiny @joekaufmanexpensify

[linhvovan29546]

Updated proposal to fix this problem

#54089 (comment)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021867633562881780913