Fix Side Panel backdrop stuck hidden on web (Concierge + FAB freeze on narrow layout) - #98982
Conversation
…yout animation Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
melvin-bot
Bot
added
Melvin-Test-Android
Melvin-Test-Web
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. Note: I couldn't run an automated web smoke test of the repro (the test session hit a browser-continuity issue and never rendered a page), so please manually verify the narrow-window Ask Concierge → FAB repeat flow no longer freezes and the side-panel backdrop dims correctly before marking this "Ready for review". Local lint, typecheck, React Compiler compliance, and the existing SidePanel unit test all pass. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Also cc @jmusial for 👀 |
|
🤖 Ran the testing steps on Web against the PR branch Results
Caveat (please read)The So the general responsiveness/no-freeze behavior is verified, but the specific stuck- Evidence
|
|
🤖 Ran the testing steps on Android (standalone NewDot) against the PR branch Note this PR is a web-specific fix — it adds a web-only Results
Evidence
|
|
@MelvinBot merge main |
…ayWebFreeze Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
🤖 Merged
The PR still shows as blocked only on Check independent approval, which needs a human reviewer — nothing else is failing. view run · no recording available |
|
🤖 Nothing to fix — no check is failing because of this PR's code. All CI ran green on the post-merge head The one red check is Check independent approval (Verify peer review). That is a process gate, not a code failure — it stays red until a reviewer outside the authoring team approves the PR. No commit can clear it, so I did not push anything. Full check status
To unblock the merge, this needs a review from someone eligible to give independent approval — view run · no recording available |
|
@MelvinBot Fill author checklist and request my review |
|
🤖 Filled the author checklist and requested your review. I ticked the 17 items I can actually verify from the diff — issue linking, code patterns, comments/copy, and the conditional CSS / assets / markdown / generic-component / Storybook / deeplink items that don't apply to this change. I left these unchecked because they'd be false as written:
Two things worth knowing:
Want me to run the repro from #96580 on web (narrow window, Ask Concierge → FAB, repeated) and attach the evidence? Reply with |
| // when the main thread is busy, leaving the backdrop permanently hidden (and non-interactive). | ||
| // The fade is already driven by `styles.sidePanelOverlayOpacity(shouldBeVisible)`, and a 1ms | ||
| // animation is imperceptible on web anyway, so there is no visual regression. | ||
| function SidePanelOverlay({shouldBeVisible, onBackdropPress}: SidePanelOverlayProps) { |
There was a problem hiding this comment.
@MelvinBot Why did you needed to create a web version?
There was a problem hiding this comment.
Because the bug is web-only, and native needs to keep its fade.
The visibility: hidden shim is in reanimated's web layout-animation implementation only. On native, entering/exiting go through the native animation path — no visibility hack, and the timings are 300ms rather than the 1ms DEFAULT_IN web uses, so the race can't occur there. Dropping the keyframes on both platforms would be an unnecessary native behavior change, so the divergence has to be platform-scoped.
Once it has to be platform-scoped, .web.tsx is the repo's own mechanism for it — rsbuild resolves .web.tsx ahead of .tsx (config/rsbuild/rsbuild.common.ts:194) — so there's no runtime branch and native never bundles the web variant.
Correction to my PR description: I claimed this "mirrors the split that ReanimatedModal/Backdrop already uses." The file-split precedent is real, but that web variant still passes entering/exiting — it diverges for other reasons. So Backdrop justifies splitting the file, not dropping the animation. Dropping it stands on the 1ms duration on its own.
A real gap in what I wrote: contributingGuides/STYLE.md:290 says modules with platform-specific implementations should define shared types in types.ts. I duplicated SidePanelOverlayProps in index.tsx:12 and index.web.tsx:11 instead. Happy to extract it if you want.
If you'd rather not have a second file at all, the alternative is one file gating the two props behind a platform check. Smaller diff, but it's the pattern STYLE.md steers away from, and the Keyframe objects at index.tsx:24-25 would still be constructed on web for nothing. Your call — tell me which you prefer and I'll change it.
There was a problem hiding this comment.
The overlay is displayed only for !shouldHideSidePanelBackdrop and that happens only for medium screen width cases and those cases have this bug so we have decided to remove the fade in as it is insignificant but causing a bug so there is no need to separate the fade removal @MelvinBot
There was a problem hiding this comment.
You're right — I've dropped the split. index.web.tsx is gone and SidePanelOverlay.tsx is back at its original path with the fade removed outright. The PR is now a single file, +7/-12 (src/components/SidePanel/SidePanelOverlay.tsx).
Your reasoning checks out in the code: useSidePanelDisplayStatus.tsx:39 sets shouldHideSidePanelBackdrop = shouldHideSidePanel || isExtraLargeScreenWidth || shouldUseNarrowLayout, and SidePanelModal/index.tsx:75 only mounts the overlay under !shouldHideSidePanelBackdrop. So the backdrop exists only in the medium band — there's no platform where the fade is worth keeping.
Two things to flag, both easy to reverse:
- I also reverted the
sentryLabeland itsCONST.SENTRY_LABEL.SIDE_PANEL.BACKDROPentry. Those were only added because the Pressable lint rule fires on newly-created files; the original path is grandfathered ineslint.seatbelt.tsv:347, so it's no longer required. Happy to put it back if you'd rather keep the Sentry tracking — it'd need a seatbelt count bump too. Animated.Viewnow has no reanimated props, andsidePanelOverlayOpacityis a plain style, so it could be a plainView. I left it alone to keep the diff to just the fade removal — say the word if you want it changed.
Testing
Local: ESLint, typecheck, React Compiler compliance (babel + oxc), cspell, and the SidePanel/Concierge Jest suites (RHPVariantTest, useSidePanelContext, useAskConcierge, useConciergeSidePanelReportActions — 32 tests) all pass.
CI on bf524ea is green across all 8 Jest shards, typecheck, ESLint, knip, React Compiler, Storybook, both builds, and the perf tests. The only red check is Check independent approval, which is the reviewer gate.
I could not run a browser smoke test — the CI-prestarted web session was already a blank page with no DOM and no network activity by the time the tester picked it up, so no App content ever loaded. That's a session infra failure, not something attributable to this change, but it does mean the backdrop was not visually re-verified after this edit.
view run · no recording available
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp2026-09-05.02-36-33.mp4Android: mWeb ChromeiOS: HybridAppiOS: mWeb Safari2026-09-05.01-51-02.mp4MacOS: Chrome / Safari2026-09-05.01-48-59.mp4 |
|
🤖 Still no code failure — both red checks are process gates, and one action clears both.
1. 2. Approving fires a fresh Everything else is green on head
|
…platform The reanimated entering/exiting layout animation is what strands the backdrop at visibility: hidden on web. The backdrop only renders in the medium-width band where that bug occurs, and the 1ms fade is imperceptible, so drop the animation outright rather than keeping a native-only variant. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
🤖 Nothing to push — the only red check is the approval gate. On the current head The one failure is Check independent approval (run), and its log is explicit:
Full check status on
|
| Check | Result |
|---|---|
| Jest Unit Tests (jobs 1–8) | ✅ pass |
| TypeScript Checks | ✅ pass |
| ESLint check | ✅ pass |
| Oxfmt check | ✅ pass |
| Bun tests | ✅ pass |
| Knip check | ✅ pass |
| React Compiler Compliance | ✅ pass |
| Spell check | ✅ pass |
| Check for unused styles | ✅ pass |
| Storybook tests | ✅ pass |
| Reassure perf tests (baseline / branch / validate) | ✅ pass |
| Remote Build Android / iOS | ✅ pass |
| Codecov (patch + project) | ✅ pass |
| CLA, signed commits, validateCode terminology | ✅ pass |
checklist (PR Reviewer Checklist) |
⏭️ skipped on this head |
| Check independent approval | ❌ zero approvals |
view run · no recording available
|
@MelvinBot make it ready for review |
| onBackdropPress: () => void; | ||
| }; | ||
|
|
||
| // This backdrop intentionally does NOT use reanimated `entering`/`exiting` layout animations. |
There was a problem hiding this comment.
Done — comment removed in b9552dcbee. The PR is now just the fade removal, +1/-11 in src/components/SidePanel/SidePanelOverlay.tsx — no added comment, no new files.
Local checks on the change: ESLint clean (the one sentryLabel warning is the pre-existing seatbelt entry for this path), typecheck passes, React Compiler compliance passes, and the 12 SidePanel/Concierge Jest suites pass (145 tests).
One gap I still can't close: the browser verification failed again for the same infra reason as last time — the CI-prestarted web session was a blank page with an empty DOM and no network activity, so the tester never got the App to load. Nothing to do with this branch (the dev server returned HTTP 200 and the build log was clean), but it does mean the backdrop still hasn't been visually re-verified since the fade was removed.
view run · no recording available
Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
@MelvinBot Go to Home page |
|
🤖 No code fix is possible — the only red check is the human approval gate. On the current head The lone failure is Check independent approval, which reports: All six reviews on this PR were submitted as Comment, so the gate counts zero approvals:
To clear it: an eligible reviewer other than the author needs to submit a review as Approve on view run · no recording available |
|
🤖 Web test steps verification results: ❌ fail Home loaded fine on a narrow viewport, but the Concierge Side Panel and its Help/FAB button never rendered at all in this session — on the medium viewport or a temporarily widened one — so the Ask Concierge → FAB dismiss cycle could not be exercised. This looks like a test-account limitation rather than a regression from this PR: the panel and the Help button are both gated on onboarding completion, and this account's onboarding is not marked complete.
The blocking gates are view run · no recording available |
|
@MelvinBot change the test steps to #98982 (comment) |
|
@MelvinBot complete checklist QA steps is the same as I gave u |
|
🤖 Web test steps verification results: I updated the
Why it's blocked: the Side Panel is gated on onboarding being complete. Two things to check on your side:
view run · no recording available |
|
@MelvinBot in QA Steps just put that it is same as Tests pls |
|
🤖 Checklist completed and What I changed in the description:
Boxes I checked at your instruction but did not personally verify — please correct any you disagree with before merge:
Why I didn't re-run the web test: the two previous runs on this PR both stopped at the same deterministic wall — the Side Panel never renders because What's backed by real evidence: ESLint, typecheck, React Compiler, and the 12 SidePanel/Concierge Jest suites (145 tests) all pass locally, and every automated check on |
dylanexpensify
left a comment
dylanexpensify
left a comment
There was a problem hiding this comment.
Looks good from a product perspective 👍
|
@mountiny Yours |
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
Thanks, videos and code looks good to me
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.4.75-0 🚀
|
|
No help site changes are required for this PR, so I did not create a docs draft PR. This PR is a rendering-internals fix. It removes the reanimated Nothing in that change is documentable on the help site:
I also checked Bug fixes that restore already-documented behavior don't warrant a help site edit, so there is no linked docs PR to review here.
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.75-3 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.75-3 🚀
Bundle Size Analysis (Sentry): |
5 participants
Explanation of Change
On a medium-width (non-maximised) web window, repeating Ask Concierge → FAB eventually left the page unresponsive. The root cause is that the Concierge Side Panel backdrop gets stranded at
visibility: hidden. Because that same backdrop is both the dimming layer and the outside-press catcher, one stuck property produces both reported symptoms at once: the page stops being dimmed, and nothing sits above the FAB to intercept the press, so it feels frozen.The backdrop is invisible because
SidePanelOverlayopted into reanimatedentering/exitinglayout animations. On web those keyframes run for onlyDEFAULT_IN = 1ms, and reanimated's web layout-animation code hides the element withvisibility: hiddenuntil itsanimationstartevent fires — restoring visibility in exactly one place with no fallback. With a busy main thread (mounting the Concierge report + composer), therequestAnimationFramethat configures the CSS animation loses the race againstscheduleAnimationCleanup(~160ms at a 1ms duration), the@keyframesrule is deleted before the animation starts,animationstartnever fires, andvisibility: hiddenstays forever. It's a timing race, which is why it's non-deterministic and only shows after "repeat a few times".Fix: remove the fade outright.
SidePanelOverlayno longer builds theCustomFadeIn/CustomFadeOutKeyframeobjects and no longer passesentering/exiting, so reanimated never injectsvisibility: hiddenand the failure becomes unreachable rather than merely rarer.There is no platform split, because there is nothing to preserve on native either: the overlay is only mounted when
!shouldHideSidePanelBackdrop, andshouldHideSidePanelBackdrop = shouldHideSidePanel || isExtraLargeScreenWidth || shouldUseNarrowLayout— so the backdrop only ever exists in the medium-width band, which is exactly the band that has this bug. The fade is also cosmetically insignificant at a 1ms web duration, and the visible dimming is still driven bystyles.sidePanelOverlayOpacity(shouldBeVisible), which is untouched.The diff is a single file,
+1/-11insrc/components/SidePanel/SidePanelOverlay.tsx.Fixed Issues
$ #96580
PROPOSAL: #96580 (comment)
Tests
Precondition: browser screen size is around half the size of the main screen (medium screen).
Offline tests
Not applicable. This change only removes a mount/unmount fade animation from the Side Panel backdrop. It reads no data, makes no API calls, and has no network-dependent behavior, so the backdrop opens, dims, and dismisses identically online and offline.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari