[HOLD for #46611][$500] App displays 'Most recent' for some time after login and also does not change background color when value is changed

[kavimuru]

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

---

Action Performed:

1. Open the app
2. Open settings->preferences->priority mode
3. Copy the URL and change value to '#focus'
4. Logout and paste the URL
5. Login and observe that for some time, value is displayed as 'Most Recent' and once it changes to '#focus', background is still highlighted for 'Most Recent'

Expected Result:

App should display correct priority mode on login and also maintain background highlight over correct value

Actual Result:

App displays 'Most Recent' as selected value even though '#focus' is current selected priority mode on login and also maintain background highlight is maintained on 'Most Recent' even when value is rectified

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.74-0
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation

focus.on.login.and.background.highlight.too.is.wrong.mp4

Recording.1604.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693324854860599

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017588740c0155fa37
• Upwork Job ID: 1706385608822575104
• Last Price Increase: 2023-10-02

[melvin-bot]

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

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

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Triggered auto assignment to @puneetlath (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh (External)

[lcsvcn]

Proposal

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

App displays 'Most recent' for some time after login and also does not change background color when value is changed

What is the root cause of that problem?

The root cause of the problem is that we currently don't display a loading while app data is still being fetched, that way the user can interact with the select list before the data is fetched from the backend.
My proposal is to add a loading screen.

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

I recommend adding the FullScreenLoadingIndicator component, similar to how it is done in the LegalNamePage.js:

App/src/pages/settings/Profile/PersonalDetails/LegalNamePage.js

Line 80 in 70e9fa0

<FullscreenLoadingIndicator style={[styles.flex1, styles.pRelative]} />

At:

App/src/pages/settings/Preferences/PriorityModePage.js

Lines 24 to 26 in 70e9fa0

	const defaultProps = {
	priorityMode: CONST.PRIORITY_MODE.DEFAULT,
	};

We change to this:

const defaultProps = {
    isLoading: true,
    priorityMode: CONST.PRIORITY_MODE.DEFAULT,
};

At:

App/src/pages/settings/Preferences/PriorityModePage.js

Lines 55 to 59 in 70e9fa0

	<SelectionList
	sections={[{data: priorityModes}]}
	onSelectRow={updateMode}
	initiallyFocusedOptionKey={_.find(priorityModes, (mode) => mode.isSelected).keyForList}
	/>

We change to this:

    {props.isLoading ? (
                <FullscreenLoadingIndicator />
            ) : (
                <SelectionList
                    sections={[{data: priorityModes}]}
                    onSelectRow={updateMode}
                    initiallyFocusedOptionKey={_.find(priorityModes, (mode) => mode.isSelected).keyForList}
                />
            )}

At:

App/src/pages/settings/Preferences/PriorityModePage.js

Lines 70 to 74 in 70e9fa0

	withOnyx({
	priorityMode: {
	key: ONYXKEYS.NVP_PRIORITY_MODE,
	},
	}),

We change to this:

withOnyx({
        isLoading: {
            key: ONYXKEYS.IS_LOADING_APP,
        },
        priorityMode: {
            key: ONYXKEYS.NVP_PRIORITY_MODE,
        },
    }),

This is a video working locally with the changes that I mention above.

Video

Screen.Recording.2023-09-25.at.16.22.34.mov

What alternative solutions did you explore? (Optional)

N/A

[hungvu193]

Proposal

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

App displays 'Most recent' for some time after login and also does not change background color when value is changed

What is the root cause of that problem?

In PriorityModePage, we're using SelectionList with priorityModes as below, while props.priorityMode is loading from server, we will use its default value first (which is CONST.PRIORITY_MODE.DEFAULT), after that when props.priorityMode is loaded now the tick mark will move to the correct value, however as we can see in this issue, its background stayed at its default value.

App/src/pages/settings/Preferences/PriorityModePage.js

Lines 29 to 35 in 3e9fdf6

	const priorityModes = _.map(_.values(CONST.PRIORITY_MODE), (mode) => ({
	value: mode,
	text: props.translate(`priorityModePage.priorityModes.${mode}.label`),
	alternateText: props.translate(`priorityModePage.priorityModes.${mode}.description`),
	keyForList: mode,
	isSelected: props.priorityMode === mode,
	}));

Let take a look at our initiallyFocusedOptionKey props in here:

App/src/pages/settings/Preferences/PriorityModePage.js

Line 61 in 3e9fdf6

initiallyFocusedOptionKey={_.find(priorityModes, (mode) => mode.isSelected).keyForList}

And here:

App/src/components/SelectionList/BaseSelectionList.js

Line 143 in 3e9fdf6

const [focusedIndex, setFocusedIndex] = useState(() => _.findIndex(flattenedSections.allOptions, (option) => option.keyForList === initiallyFocusedOptionKey));

initiallyFocusedOptionKey is calculated for the focusedIndex, and because it's state, it can't be updated even when initiallyFocusedOptionKey, that's why our background color isn't changed even when props.priorityMode is loaded. That's the root cause.

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

Add an useEffect to update our focusedIndex when the initiallyFocusedOptionKey changed inside our BaseSelectionList:

    useEffect(() => {
        setFocusedIndex(_.findIndex(flattenedSections.allOptions, (option) => option.keyForList === initiallyFocusedOptionKey))
    }, [initiallyFocusedOptionKey, flattenedSections])

What alternative solutions did you explore? (Optional)

Add an loading while our props.priorityMode is loading, by this way we can get the correct focusedIndex when the page is rendered.

// we can use isLoadingApp as well
{isLoadingReportData ? <FullScreenLoadingIndicator style={[styles.flex1, styles.pRelative]} /> : <SectionList ...  />}

[kadiealexander]

@fedirjh any thoughts on the proposals here?

[fedirjh]

Can we put this issue on hold for #27767 ?

@hungvu193 proposal makes sense to me, but it seems that BaseSelectionList is currently undergoing refactoring and is very close to being merged. The ongoing refactoring does indeed impact the part @hungvu193 has mentioned in his proposal.

[lcsvcn]

@fedirjh why the solution I proposed doesn't make sense to you?

The solution I proposed is how it is done across all other sections of the app, l still think it is more appropriated fix to this bug, because it solves this issue and the one I reported in the chat. Another one that raises when user has a slow network connection, and it is how it is done across the app in other sections.

Similar issues on other parts of the app with similar solution that I propose:

Address page:
#27814 (comment)

Display Name page:
#28077 (comment)

As I reported here: https://expensify.slack.com/archives/C049HHMV9SM/p1695655428909709

"No loading indicating that it is fetching the data. While loading, user can select another option and if he is using a slow network, he will be able to save a "new priority mode", but when finish loading it will show the server saved priority mode (See video)"

So I agree that @hungvu193 solves the issue, but the issue I reported would be still present with his solution, so maybe we need to address the issue I reported in a different bug? Because I was told that this was a duplicate, but seems like it was not in the end.

To be honest, I think the bug I reported is not the same as this one, but if we fix the bug I reported, it will automatically solve this issue

[fedirjh]

why the solution I proposed doesn't make sense to you?

hey @lcsvcn Please check the expected result in the OP.

[fedirjh]

@kadiealexander What is the expected result when we initially open the priority mode preference page?

1. Should we display the default data until the server data is synced?
2. Should we display a loading indicator until the data is ready?

I am leaning toward the first second option as it will prevent the interaction while the data is loading.

---

@hungvu193 For the initiallyFocusedOptionKey, is it possible to replicate this bug in another place across the App?

[hungvu193]

@hungvu193 For the initiallyFocusedOptionKey, is it possible to replicate this bug in another place across the App?

It will happen any where that we use initiallyFocusedOptionKey, because it's loaded once

[hungvu193]

@kadiealexander What is the expected result when we initially open the priority mode preference page?

1. Should we display the default data until the server data is synced?

2. Should we display a loading indicator until the data is ready?

For more context, 1 was the way we handled it before, it's only broken after we refactored the BaseOptionList

[lcsvcn]

@fedirjh what is OP?

[fedirjh]

what is OP?

It's the original post.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[kadiealexander]

@kadiealexander What is the expected result when we initially open the priority mode preference page?

Should we display the default data until the server data is synced?
Should we display a loading indicator until the data is ready?

I am leaning toward the second option as it will prevent the interaction while the data is loading.

@fedirjh Agreed, I don't think we should show default data and have it be interactable if it's possibly going to change after it loads.

[flodnv]

Still on hold, not a priority at this time.

[kadiealexander]

Same as above.

[kadiealexander]

Still on hold.

[kadiealexander]

Same as above.

[kadiealexander]

@flodnv can we close this?

[dhanashree-sawant]

Hi @kadiealexander, @flodnv is this issue eligible for reporting bonus?

[flodnv]

This is now on hold for #46611

[kadiealexander]

Still on hold.

[jasonp2203]

ITS BECAUSE I DON'T HAVE A LOCAL BANK

[melvin-bot]

📣 @jasonp2203! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[flodnv]

Still holding for #46611, we're getting closer.

[kadiealexander]

@flodnv should we close based on this?

[flodnv]

Probably, yes.