[$250] The focused background isn't updated when the selected option is updated from BE side #54089

m-natarajan · 2024-12-13T01:05:31Z

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] 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:

In the device 1, go to Settings> Preferences > Priority mode
In the device 2, go to Settings> Preferences > Priority mode
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?

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

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @DylanDylann

melvin-bot · 2024-12-13T01:05:41Z

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 · 2024-12-13T01:55:57Z

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 · 2024-12-13T02:45:36Z

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 · 2024-12-13T03:32:29Z

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?

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 · 2024-12-13T04:16:54Z

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

DylanDylann · 2024-12-13T04:18:12Z

@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 · 2024-12-13T05:23:19Z

Updated proposal to fix this problem

#54089 (comment)

melvin-bot · 2024-12-13T18:11:56Z

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

melvin-bot · 2024-12-13T18:12:00Z

Current assignee @DylanDylann is eligible for the External assigner, not assigning anyone new.

mountiny · 2024-12-13T18:12:03Z

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

Make sure to add tests too

linhvovan29546 · 2024-12-14T05:33:32Z

Updated proposal to add automated tests

#54089 (comment)

m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 13, 2024

melvin-bot bot assigned michaelkwardrop Dec 13, 2024

mountiny assigned DylanDylann Dec 13, 2024

mountiny added the External Added to denote the issue can be worked on by a contributor label Dec 13, 2024

melvin-bot bot changed the title ~~The focused background isn't updated when the selected option is updated from BE side~~ [$250] The focused background isn't updated when the selected option is updated from BE side Dec 13, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 13, 2024

michaelkwardrop added this to [#whatsnext] #quality Dec 13, 2024

michaelkwardrop moved this to LOW in [#whatsnext] #quality Dec 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$250] The focused background isn't updated when the selected option is updated from BE side #54089

[$250] The focused background isn't updated when the selected option is updated from BE side #54089

m-natarajan commented Dec 13, 2024 •

edited by michaelkwardrop

Loading

melvin-bot bot commented Dec 13, 2024

nyomanjyotisa commented Dec 13, 2024

linhvovan29546 commented Dec 13, 2024 •

edited

Loading

daledah commented Dec 13, 2024 •

edited

Loading

DylanDylann commented Dec 13, 2024

DylanDylann commented Dec 13, 2024 •

edited

Loading

linhvovan29546 commented Dec 13, 2024

melvin-bot bot commented Dec 13, 2024

melvin-bot bot commented Dec 13, 2024

mountiny commented Dec 13, 2024

linhvovan29546 commented Dec 14, 2024

[$250] The focused background isn't updated when the selected option is updated from BE side #54089

[$250] The focused background isn't updated when the selected option is updated from BE side #54089

Comments

m-natarajan commented Dec 13, 2024 • edited by michaelkwardrop Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 13, 2024

nyomanjyotisa commented Dec 13, 2024

Proposal

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

What is the root cause of that problem?

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

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

What alternative solutions did you explore? (Optional)

linhvovan29546 commented Dec 13, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

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

What alternative solutions did you explore? (Optional)

daledah commented Dec 13, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

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

What alternative solutions did you explore? (Optional)

DylanDylann commented Dec 13, 2024

DylanDylann commented Dec 13, 2024 • edited Loading

linhvovan29546 commented Dec 13, 2024

melvin-bot bot commented Dec 13, 2024

melvin-bot bot commented Dec 13, 2024

mountiny commented Dec 13, 2024

linhvovan29546 commented Dec 14, 2024

m-natarajan commented Dec 13, 2024 •

edited by michaelkwardrop

Loading

linhvovan29546 commented Dec 13, 2024 •

edited

Loading

daledah commented Dec 13, 2024 •

edited

Loading

DylanDylann commented Dec 13, 2024 •

edited

Loading