[$250] Group - ‘Notification preferences’ page transition animation is broken

[izarutskaya]

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: v1.4.63-0
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Create a group chat
2. Click on chat header > Settings > Click ‘Notify me about new replies’

Expected Result:

There should be animation when navigating to ‘Notification preferences’ page

Actual Result:

There is no animation when navigating to ‘Notification preferences’ page

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

Bug6453305_1713425823146.Screen_Recording_2024-04-18_at_10.31.53_in_the_morning.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0196a30d8dae037242
• Upwork Job ID: 1781029201239011328
• Last Price Increase: 2024-04-18

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @danieldoglas (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[izarutskaya]

@sakluger I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[izarutskaya]

We think this issue might be related to the #vip-vsb.

[izarutskaya]

Production

bandicam.2024-04-18.14-19-01-918.mp4

[danieldoglas]

I'm not sure if we should block a deploy on this.

[danieldoglas]

Couldnt find any obvious changes that caused this

[marcaaron]

I agree. It seems very minor.

[marcaaron]

since it's related to Group Chats I am willing to take it from ya @danieldoglas

[melvin-bot]

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

[melvin-bot]

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

[allgandalf]

Cannot reproduce on staging :)

[shahinyan11]

Proposal

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

Group - ‘Notification preferences’ page transition animation is broken

What is the root cause of that problem?

In BaseListItem component we use useSyncFocus hook to focus list item. And in this case, we focus the element while the navigation animation is not yet complete, which results in a braking animation.

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

Move this code this code into the InteractionManager.runAfterInteractions callback

InteractionManager.runAfterInteractions(() => {
    ref.current?.focus();
});

What alternative solutions did you explore? (Optional)

[danieldoglas]

Assigned yo you @marcaaron , thanks!

[paultsimura]

Thanks for your proposal @shahinyan11 – unfortunately, the offending PR is still in the regression period and should be handled by the authors: @WojtekBoman and @fedirjh.

[tienifr]

Proposal

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

There're 2 bugs in the Notification preferences feature, both relating to the option focusing:

1. There is no animation when navigating to ‘Notification preferences’ page
2. When coming to ‘Notification preferences’ page and press "ArrowDown" to navigate (the default selected option will be Immediate), the Immediate still has browser focus, but there's no item-focus. In this case the first item in the list should be focused because we have cyclic traversal by default in arrows navigation

What is the root cause of that problem?

1. In here, we're manually trigger .focus on the item if it's app-focused, this is so that the item will have browser focus too. But this manual focusing will interfere with the screen transition, causing the animation to be skipped and the page shows up immediately.
2. In here, we're setting the wrong maxIndex for the useArrowKeyFocusManager. The maxIndex's purpose is so that we know whether we reached the edge of the list and should go to top/bottom in a cyclic manner. If the list has 3 items (as in notification preferences), the maxIndex should be 2 (there's item 0, 1, 2). However we're setting the maxIndex to the length of the list (3 here), leading to the focus being moved outside of the screen when we do ArrowDown when we're at the last item of the list (same as when we're on top of the list and ArrowUp)

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

1. In here, we should only focus after the screen transition was all done. We can apply the same approach we already use for useAuthFocusInput here, which is to wait a specified timeout after the current screen is focused.

Or we can put the focusing here inside InteractionManager.runAfterInteractions, but this might not be as reliable

2. In here, update flattenedSections.allOptions.length to flattenedSections.allOptions.length - 1

What alternative solutions did you explore? (Optional)

NA

[tienifr]

@paultsimura I think this can be handled externally since it has Help Wanted label. If not, we should not have put Help Wanted on it since this was a deploy blocker so it of course comes from a recent PR (by definition).

What do you think?

[paultsimura]

@marcaaron WDYT?
#40426 (comment)
#40426 (comment)

[WojtekBoman]

Hi! I'll take a look at it :)

[marcaaron]

Sounds good to me @paultsimura.

@tienifr I can't think of why adding the Help Wanted label would mean that the original authors are not responsible for fixing. But if you are passionate about discussing it please email [email protected] or bring it up in the Slack channel.

[fedirjh]

I think this can be closed.

[marcaaron]

Thanks!