[HOLD on PR #34196] [$500] Profile -The preselected pronoun isn't highlighted by default

[lanitochka17]

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: 1.4.27-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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Action Performed:

1. Open Expensify App
2. Log in
3. Navigate to settings by tapping on the user avatar
4. Tap on Profile and then Pronouns
5. Search and select a pronoun
6. Tap again Pronouns
7. Verify if the preselected pronoun is highlighted

Expected Result:

The selected pronouns should have been highlighted by default when there is no searching performed (as in the timezone selection)

Actual Result:

Only green checkmark exist in the preselected pronoun, but it is not highlighted (both if there is no search or after search)

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6346771_1705598027784.mobizen_20240118_191252.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01af844a0a64bbba29
• Upwork Job ID: 1748037184016420864
• Last Price Increase: 2024-01-25

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[aeioual]

Proposal

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

The preselected pronoun isn't highlighted by default

What is the root cause of that problem?

In the BaseSelectionList component, here we forget to reverify whether initiallyFocusedOptionKey is available.

App/src/components/SelectionList/BaseSelectionList.js

Lines 369 to 380 in bcd783f

	useEffect(() => {
	// do not change focus on the first render, as it should focus on the selected item
	if (isInitialSectionListRender) {
	return;
	}
	// set the focus on the first item when the sections list is changed
	if (sections.length > 0) {
	updateAndScrollToFocusedIndex(0);
	}
	// eslint-disable-next-line react-hooks/exhaustive-deps
	}, [sections]);

This always defaults the updateAndScrollToFocusIndex to the Index 0

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

we should make the following changes before setting the updateAndScrollToFocusIndex value:

if (sections.length > 0) {

  const currentSelection = _.findIndex(
    flattenedSections.allOptions, (option) =>
    option.keyForList === initiallyFocusedOptionKey)

  updateAndScrollToFocusedIndex(currentSelection ?? 0);
}

The condition currentSelection ?? 0 checks if currentSelection is null or undefined (indicating that the specified option was not found), it defaults to 0.

Test branch with implemented functionality can be found here: Test Branch

What alternative solutions did you explore? (Optional)

N/A

[esh-g]

Proposal

Please re-state the issue we are trying to solve

The preselected pronoun isn't highlighted by default after searching.

What is the root cause of this issue?

The root cause is that we don't recheck if initallyFocusedOptionKey is present after the first render. So after changing search text, we just focus to the 0 index and don't look for that initallyFocusedOptionKey

What changes should be made to fix this?

We should modify this:

App/src/components/SelectionList/BaseSelectionList.js

Lines 375 to 378 in bcd783f

	// set the focus on the first item when the sections list is changed
	if (sections.length > 0) {
	updateAndScrollToFocusedIndex(0);
	}

To:

// set the focus on the first item when the sections list is changed
if (sections.length > 0) {
    const initallyFocused = _.findIndex(flattenedSections.allOptions, (option) => option.keyForList === initiallyFocusedOptionKey)
    updateAndScrollToFocusedIndex(initallyFocused ?? 0);
}

This will check for the initialItem every time sections update.

Result

Screen.Recording.2024-01-18.at.11.27.32.PM.mov

[ikevin127]

Might be regression from #30438.

A PR #34196 is already opened that will tackle that regression. Another issue #34300 that I was told will be fixed by the opened PR.

[ZhenjaHorbach]

Proposal

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

Profile -The preselected pronoun isn't highlighted by default

What is the root cause of that problem?

The main problem with issue is that when we open pronoun screen
we have a case where we initially have an empty array
And we keep the index -1

App/src/components/SelectionList/BaseSelectionList.js

Line 158 in bcd783f

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

But then when we update section list we can't change index
Because we still have isInitialSectionListRender is true

App/src/components/SelectionList/BaseSelectionList.js

Lines 369 to 380 in bcd783f

	useEffect(() => {
	// do not change focus on the first render, as it should focus on the selected item
	if (isInitialSectionListRender) {
	return;
	}
	// set the focus on the first item when the sections list is changed
	if (sections.length > 0) {
	updateAndScrollToFocusedIndex(0);
	}
	// eslint-disable-next-line react-hooks/exhaustive-deps
	}, [sections]);

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

To fix this issue we can just update the code and update the index when we still have isInitialSectionListRender === true

        if (isInitialSectionListRender) {
            setFocusedIndex(flattenedSections.allOptions.findIndex((option) => option.keyForList === initiallyFocusedOptionKey));
            return;
        }

App/src/components/SelectionList/BaseSelectionList.js

Lines 371 to 373 in bcd783f

	if (isInitialSectionListRender) {
	return;
	}

Using the logic from focusedIndex initialization

App/src/components/SelectionList/BaseSelectionList.js

Line 158 in bcd783f

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

What alternative solutions did you explore? (Optional)

NA

[DylanDylann]

Proposal

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

The selected pronouns should have been highlighted by default when there is no searching performed (as in the timezone selection)

Currently the selected option isn't highlighted right after user go to pronouns page (don't type anything on search input)

In addition, When user type in the search input, It makes sense to focus on the first option

What is the root cause of that problem?

Let's see here

App/src/components/SelectionList/BaseSelectionList.js

Line 158 in 991a4e2

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

We are setting the default value for focusedIndex in the param of useState hook, with this approach the default value only be calculated in the first render. But in the first render flattenedSections.allOptions is empty

Even though, after flattenedSections.allOptions is updated the focusedIndex will not be calculated again

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

We should use a useEffect to calculate focusedIndex again when flattenedSections.allOptions change

App/src/components/SelectionList/BaseSelectionList.js

Line 158 in 991a4e2

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

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

What alternative solutions did you explore? (Optional)

NA

Result

[fedirjh]

This will be fixed with #34196, screenshot from PR branch :

CleanShot.2024-01-24.at.09.56.03.mp4

[melvin-bot]

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

[fedirjh]

On hold for #34196

[muttmuure]

Not overdue

[muttmuure]

Held

[melvin-bot]

@fedirjh, @muttmuure Whoops! This issue is 2 days overdue. Let's get this updated quick!

[muttmuure]

@fedirjh any closer on the linked PR?

[muttmuure]

Still waiting on the linked PR

[melvin-bot]

@fedirjh, @muttmuure Whoops! This issue is 2 days overdue. Let's get this updated quick!

[fedirjh]

#34196 is deployed to staging.

[melvin-bot]

@fedirjh, @muttmuure Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[fedirjh]

@muttmuure This issue was fixed with #34196, can you please retest and close?

[muttmuure]

Yep, looks good