[$1000] Web – Announce Room - Inconsistency in highlighting the selected option.

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Click on "Settings."
2. Click on "Preferences."
3. Click on "Priority mode" and "Language."
4. Notice that the selected option in "Priority mode" and "Language" is highlighted.
5. Open any "announce room."
6. Open "Details."
7. Click on "Settings."
8. Click on "Who can post" and "Notify me."
9. Notice that the selected option is not highlighted.

Expected Result:

The selected option in "Who can post" and "Notify me" should be highlighted to maintain consistency among all pages.

Actual Result:

The selected option is not highlighted.

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.38-3
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

1.mp4

Recording.3531.mp4

Expensify/Expensify Issue URL:
Issue reported by: @usmantariq96
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688967984457429

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01422c202471bfda96
• Upwork Job ID: 1681657941126709248
• Last Price Increase: 2023-07-19

[melvin-bot]

Triggered auto assignment to @JmillsExpensify (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

[DanutGavrus]

Proposal

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

Web – Announce Room - Inconsistency in highlighting the selected option.

What is the root cause of that problem?

1. For Settings -> Preferences -> Priority mode & Language we are using SelectionListRadio which has initiallyFocusedOptionKey property;
2. In Announce Rooms -> Details -> Settings -> Who can post & Notify me we are using OptionsList which does not have the initially focused functionality.

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

We should either:
I. Replace the OptionsList with SelectionListRadio, but keep in mind that SelectionListRadio does not have some functionalities which we'll lose such as shouldHaveOptionSeparator which adds a green line between rows.
II. Implement the initially focused functionality for the OptionsList same as it works for the SelectionListRadio.

What alternative solutions did you explore?

Didn't investigate enough, but I'm curious if we would be able to use just one list implementation everywhere, and set functionalities based on props. For example, OptionsList at /settings/preferences/language and SelectionListRadio at /r/YOUR-WORKSPACE-ID/settings/notification-preferences look very similar. I think this would be a bigger refactor if possible.

[GItGudRatio]

Proposal

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

The Option List selection is inconsistent in the "Who can Post" and "Notify Me" pages.

What is the root cause of that problem?

We recently adopted the use of the SelectionListRadio component for selecting single options from a list. The WriteCapabilityPage and NotificationPreferencePage were missed during the refactor however, causing the inconsistent design.

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

We need to replace the OptionsList with SelectionListRadio in both these pages. To do so in each of these pages,

Notification Preferences Page

1. In the notificationPreferenceOptions, we will remove customIcon and boldStyle and add a new property isSelected which has the value as props.report.notificationPreference === key
2. Replace OptionsList with SelectionListRadio and remove all props except sections and onSelectRow.
3. Add a new prop initiallyFocusedOptionKey with value _.find(notificationPreferenceOptions, (preference) => preference.isSelected).keyForList.
4. Remove all unused variables and imports.

This will ensure consistent behaviour on the Notification Preferences page.

Write Capability Page

Similar steps as above but in writeCapabilityOptions, we use it's respective condition for determining selection.

What alternative solutions did you explore? (Optional)

N/A

[dukenv0307]

This bug is reported before and we decide to close because we had an issue to refactor optionSelector

[JmillsExpensify]

@dukenv0307 Do you mind pointing me to that convo?

[dukenv0307]

I don't remember exactly. @shawnborton Could you help to confirm?

[shawnborton]

Yeah, @thiagobrez was working on this and can provide some PR links.

[thiagobrez]

Hey!

The selection list refactor tracker issue is here: #11795

Phase 1 - single selection lists:

• Issue: [HOLD for payment 2023-07-17] Selection List Refactor: Radio Button List #20352
• PR: refactor: SelectionListRadio #21048

Phase 2 - multi selection lists:

• Issue: [HOLD for payment 2023-09-04] Selection List Refactor: Checkbox List #20353
• PR: refactor: SelectionList multiple selection #22622

Phase 3 - simple selection lists:

• Issue: LOW: [$500] Selection List Refactor: Simple Selection List #20354
• PR: not started yet

---

So what happened seems to be that we forgot about some list cases. I was following the cases listed in this comment from the tracker issue, so that's why the Announce room was forgotten.

Before this refactor, we had this component hierarchy:

• OptionsSelector parent of OptionsList parent of OptionRow

These 3 component were being used interchangeably. For example, sometimes OptionsList being rendered on it's own without OptionsSelector (which is it's parent). That, in addition to a bunch of styling props, made all lists just a little bit different from each other.

We're aiming to replace all usages of OptionsSelector, OptionsList and OptionRow with the new SelectionList.

In main, you will still find SelectionListRadio, which is the result of the Phase 1 PR. In Phase 2 PR, I renamed it to SelectionList and iterated on top of it making it also able to render multiple selection lists (with checkboxes instead of checkmarks). On Phase 3, I'm thinking I'll come up with a new component instead of iterating on top of this same one again because the requirements seem quite different, let's see.

TL;DR:

• We want to replace all usages of OptionsSelector, OptionsList and OptionRow with the new SelectionList
• Lists that are not using this yet is because we simply forgot to track 🤷🏻 , or by some decision made
• Yes, some styling props (such as shouldHaveOptionSeparator) don't exist anymore, and that's intended, to standardise the look and feel of same-purpose lists

[thiagobrez]

That said, I believe both Proposals from @DanutGavrus (point I) and @GItGudRatio are correct. You can either raise a PR in this issue, or I can add those to my Phase 2 PR which is currently open, however you prefer.

cc @shawnborton ^

[GItGudRatio]

I'd be happy to raise a PR within the hour if I am assigned to the issue, I've got the code commited already. :)

[DanutGavrus]

@thiagobrez As my Proposal's variant I and @GItGudRatio's Proposal are the same, shouldn't I be assigned here?

[GItGudRatio]

We commented at pretty much the same time and I added the exact implementation details as well, please do take that into consideration.

We could also work together as one of us could raise the PR and the other could review and I'd be happy to split the bounty with @DanutGavrus.

[thiagobrez]

I'm not from EXFY guys, I can't make those calls :)

I believe you want @JmillsExpensify here

[melvin-bot]

@JmillsExpensify Eep! 4 days overdue now. Issues have feelings too...

[JmillsExpensify]

Let's triage the bug first and C+ can make that call.

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[Santhosh-Sellavel]

Sorry bit late here I agree with @thiagobrez, we have ongoing migrations so we would handle it there, let's close this one thanks!

cc: @JmillsExpensify @rushatgabhane

[DanutGavrus]

@Santhosh-Sellavel @JmillsExpensify @rushatgabhane If this issue was External and if my Proposal was left before the same change was applied in the migrations, will I be eligible for the bounty?
I think I saw something similar in another Issue recently, but can't find it easily so I thought of asking. Thanks!

[Santhosh-Sellavel]

@Santhosh-Sellavel @JmillsExpensify @rushatgabhane If this issue was External and if my Proposal was left before the same change was applied in the migrations, will I be eligible for the bounty? I think I saw something similar in another Issue recently, but can't find it easily so I thought of asking. Thanks!

We are already in the process of migrating to the selection list. So the proposal doesn't qualify for a bounty.

[melvin-bot]

@JmillsExpensify @rushatgabhane this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@JmillsExpensify, @rushatgabhane Huh... This is 4 days overdue. Who can take care of this?

[JmillsExpensify]

Thanks @Santhosh-Sellavel! Closing this one out.