Repository navigation
TASK-23054 drawer CTAs keep their bottom shadow; gate the class in CI - #3487
Conversation
Staging QA: the "Update residence" CTA in the Accounts page's "Residence required" drawer had its offset shadow cut flat at the bottom. Since ui#3459 removed the caption under it, the CTA ends the drawer content, and the DrawerContent scroll box (overflow-auto) clipped at its own bottom edge. The panel's pb-4 sits outside that box, so it never helped. CorridorGateDrawer on Add money had the same cut. The fix is once, in DrawerContent: an 8px ::after block (the deepest button shadow) inside the scroll box, given back by -mb-2 on its wrapper so no sheet changes height or spacing. A pseudo-element, so a caller's scrollAreaClassName pb-* cannot drop it; pb-safe-bottom still applies below. Guard: the DS showcase's shadow-clip detector moves to e2e/utils and a new spec runs it over every /dev/surfaces surface, the home drawers and the tap-opened account drawers at 375x667. It runs in press-contract, so a clipped CTA shadow now fails ci-success instead of reaching QA.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: peanutprotocol/peanut-ui/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Code-analysis diffPainscore total: 8968.88 → 8968.56 (-0.32) 🆕 New findings (15)
✅ Resolved (14)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
The workflow change trips the backdoor guard, which waits for a non-author read. The fix and its spec land tonight; the gate follows in a separate PR.
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No actionable findings in the drawer shadow reserve or its browser sweep.
Checked clean
- Reviewed the exact merge-base patch and shared DrawerContent behavior, including panel padding, capped scroll height, safe-area padding, and caller scroll classes.
- Reviewed shadow detector extraction and new drawer/fixture cases; the new sweep is advisory at this head, with required CI wiring explicitly deferred to a separate PR.
- Exact-head available checks included successful guard and backdoor scan; capture and preview checks were still running when reviewed. No full unit or browser-suite conclusion was available.
Security review: did not run — this change has no security, privacy or money surface, so it was not asked. This review is one reviewer short.
Third opinion: did not run — claude-api_error. This review is one reviewer short.
Exact head: c6aefa1b9746 · Context: repo · Took 6m
Hugo's call: land the fix and its gate together rather than split them. A clipped CTA shadow shows only in real layout and has shipped twice.
|
1 similar comment
|
There was a problem hiding this comment.
Chip review — no blocking findings — this is not an approval
No actionable findings in the drawer shadow fix or its required CI gate.
Checked clean
- Re-reviewed the exact-head merge-base patch and the new workflow delta: press-contract runs both Playwright specs sequentially, and ci-success already requires press-contract.
- Checked the exact-head guard log: the backdoor scan was clean; guard fails intentionally because the PR changes a guarded workflow file.
- Required build, unit, and press-contract checks had not completed at review time; their outcomes remain unverified.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion: did not run — it reads only the first review of a pull request; the first reviewer checks later rounds. This review is one reviewer short.
Exact head: bd1ee1d19eb4 · Context: repo · Took 6m
|
English · Español · Español (Argentina) · Português (Brasil) After merge: 37ff433 → ebdfc5a. Capture complete in all locales. |
Bug (staging QA): in the Accounts page's "Residence required" drawer, the bottom of the "Update residence" CTA's offset shadow was cut off flat. The "Verify identity first" drawer on Add money had the same cut.
Root cause: since #3459 removed the caption under the CTA, the CTA ends the drawer content.
DrawerContent's scroll box isoverflow-auto, so it clips at its own bottom edge. The panel'spb-4(ClosedRowDrawer, CorridorGateDrawer) is outside that box and never protected the shadow. #3019 fixed the same problem for the left and right edges only.Fix, once, in
Global/Drawer:::afterblock (the deepest button shadow,shadowSize="8") inside the clip box. It is a pseudo-element, so a caller'sscrollAreaClassName="pb-*"cannot remove it (KycStatusDrawer passespb-12).-mb-2, which gives the 8px back. No drawer changes height or spacing. The only visible difference: a drawer already at itsmax-h-[80vh]cap sits 8px lower, and its content ends in the same place.pb-safe-bottomstays on the scroll box below the reserve, so the native inset still applies.Guard:
e2e/utils/shadow-clip.ts: the DS showcase's detector, moved out ofds-shadow-clip.spec.tsso both specs share it. It also takes an optionalscope(open dialog only) andviewportClips.e2e/flows/drawer-shadow-clip.spec.tschecks 87 cases at 375x667. It covers every renderable/dev/surfacessurface, the three home drawers, and the account drawers that open from a tap (closed row ARS on Accounts and on Add money, unlock BRL/USD, and the EUR corridor gate). It scrolls each dialog to the end, then fails when a button's shadow sits outside its nearest clipping ancestor or off screen.press-contractjob, so a clipped CTA shadow failsci-success. The job timeout goes from 20 to 25 min. This changes.github/workflows/tests.yml, so the backdoorguardcheck stays red until someone other than the author reads that file.DrawerContentchecks the reserve.Verified locally (preview build,
next start):prettier --check,tsc --noEmitand jest--runInBand(772 suites) pass.