[$1000] Web - The focus on timezone is highlighted in full in compared to year option on date of birth

[kbecciv]

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. Go to staging dot on web chrome
2. Settings > Profile >Personal details
3. Select date of birth and select the year field and notice that when you hover over the date years, the focus is not fully covered
4. But now go back and select the timezone and hover over the different options and notice that the timezone options focus are fully covered
5. Thus, there's inconsistency between two areas in the same application

Expected Result:

The focus on timezone should not be highlighted in full in a similar way how the focus on the year option on date of birth does to maintain consistency OR vice versa

Actual Result:

The focus on timezone is highlighted in full in compared to year option on date of birth

Workaround:

Unknown

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.27-6

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

error-2023-06-14_21.29.21.1.mp4

Recording.3197.mp4

Expensify/Expensify Issue URL:

Issue reported by: @priya-zha

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686757443117449

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0192d5df321e7923f8
• Upwork Job ID: 1671654443150655488
• Last Price Increase: 2023-06-21

[melvin-bot]

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

[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

[chiragxarora]

Proposal

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

Inconsistent focus on timezone is highlighted in full in compared to year option on date of birth

What is the root cause of that problem?

Root cause is the horizontal padding (as ph5) given as contentContainerStyles prop to <OptionsSelector/> in YearPickerPage whereas we don't have the similar padding in the TimezoneSelectPage

App/src/pages/YearPickerPage.js

Lines 105 to 119 in d43756a

	<OptionsSelector
	textInputLabel={this.props.translate('yearPickerPage.selectYear')}
	onChangeText={this.filterYearList}
	keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
	maxLength={4}
	value={this.state.inputText}
	sections={[{data: this.state.yearOptions, indexOffset: 0}]}
	onSelectRow={(option) => this.updateSelectedYear(option.value)}
	headerMessage={headerMessage}
	initiallyFocusedOptionKey={this.currentYear.toString()}
	hideSectionHeaders
	optionHoveredStyle={styles.hoveredComponentBG}
	shouldHaveOptionSeparator
	contentContainerStyles={[styles.ph5]}
	/>

App/src/pages/settings/Profile/TimezoneSelectPage.js

Lines 131 to 141 in 8ba2234

	<OptionsSelector
	textInputLabel={this.props.translate('timezonePage.timezone')}
	value={this.state.timezoneInputText}
	onChangeText={this.filterShownTimezones}
	onSelectRow={this.saveSelectedTimezone}
	optionHoveredStyle={styles.hoveredComponentBG}
	sections={[{data: this.state.timezoneOptions, indexOffset: 0, isDisabled: this.timezone.automatic}]}
	shouldHaveOptionSeparator
	safeAreaPaddingBottomStyle={safeAreaPaddingBottomStyle}
	initiallyFocusedOptionKey={_.get(_.filter(this.state.timezoneOptions, (tz) => tz.text === this.timezone.selected)[0], 'keyForList')}
	/>

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

For solving this we have two options:

• we could either remove the horizontal padding from the YearPickerPage
• or we could add the same padding to the TimezoneSelectPage

We can do either of these depending upon the ideal desired behavior, as in the OP's bug report expected result says or vice versa

EDIT: (No other proposal has mentioned this till the time of edit)

Pronouns page also does not use the padding so we have 2 places with no padding and 1 place with padding. (mentioning for consistent change after the bug is fixed)

Results

OPTION 1:

bandicam.2023-06-20.22-40-22-904.mp4

OPTION 2:

bandicam.2023-06-20.22-39-04-469.mp4

bandicam.2023-06-20.18-52-15-569.mp4

What alternative solutions did you explore? (Optional)

None

[aman-atg]

Proposal

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

Inconsistent : The focus on timezone is highlighted in full in compared to year option on date of birth

What is the root cause of that problem?

The issue arises from the usage of the contentContainerStyles prop in the OptionsSelector component in the YearPickerPage.js file. The contentContainerStyles prop is applied to the options selector, causing the inconsistency in the highlighting style.

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

To solve the problem, we should remove the contentContainerStyles prop from the OptionsSelector component in the YearPickerPage.js file here

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[lschurr]

@sobitneupane can you review these proposals?

[aliammar1995]

Proposal

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

Inconsistent : The focus on timezone is highlighted in full in compared to year option on date of birth

What is the root cause of that problem?

It is because in OptionsSelector of TimezoneSelectPage component, we did
not put custom style as prop (contentContainerStyles)

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

We have to add prop contentContainerStyles as a prop OptionsSelector in. TimezoneSelectPage component.

<OptionsSelector textInputLabel={this.props.translate('timezonePage.timezone')} value={this.state.timezoneInputText} onChangeText={this.filterShownTimezones} onSelectRow={this.saveSelectedTimezone} optionHoveredStyle={styles.hoveredComponentBG} sections={[{data: this.state.timezoneOptions, indexOffset: 0, isDisabled: this.timezone.automatic}]} shouldHaveOptionSeparator safeAreaPaddingBottomStyle={safeAreaPaddingBottomStyle} initiallyFocusedOptionKey={_.get(_.filter(this.state.timezoneOptions, (tz) => tz.text === this.timezone.selected)[0], 'keyForList')} contentContainerStyles={[styles.ph5]} />

It worked well.

[sobitneupane]

@chiragxarora's proposal looks good to me.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @NikkiWines, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[chiragxarora]

Hey @sobitneupane which option are we going with?
No padding in yearpicker or adding padding to timezone?
Removing padding from yearpicker page looks like a better option to me personally

[sobitneupane]

No padding in yearpicker

[chiragxarora]

PR is ready for review @sobitneupane
@NikkiWines you may assign me now for the issue

[NikkiWines]

Yep, agreed we can use @chiragxarora's proposal in this case

[melvin-bot]

📣 @chiragxarora You have been assigned to this job by @NikkiWines!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[dhairyasenjaliya]

@shawnborton are we getting rid of the separator line for this page as well?

[shawnborton]

I think so - basically all list views are getting refactored. Does that sound right @thiagobrez?

[chiragxarora]

Are the reviewers not taken from the issue associated? I see someone else assigned for the review on PR
Also, do I need to remove separator for pages of this issue?
@shawnborton

[shawnborton]

Honestly I think we can just close this one out because the refactor is coming. Let's wait for Thiago to confirm.

[chiragxarora]

Isn't the issue here different although for the same page? just asking

[thiagobrez]

I think so - basically all list views are getting refactored. Does that sound right @thiagobrez?

@shawnborton That's correct. We currently have a variety of lists being rendered by different components with different styles. In this tracker issue we discussed about it, and we're creating whole new components (here, here and here) to make it easier to use from a developer perspective, and maintain consistency from a design perspective

Both the Timezone list and the Year Picker list were already addressed in the first PR. The separator lines were also removed: #21048

I don't think it's worth fixing OptionsSelector, OptionsList or OptionRow components as they're getting completely replaced soon

[shawnborton]

Awesome thanks for confirming. So sounds like maybe we just close this one out then.

[NikkiWines]

Agreed, let's close this out for now 👍