PLASMA-5874: overlay click handler for Sheet and BottomSheet - #3021
PLASMA-5874: overlay click handler for Sheet and BottomSheet#3021vmytnik wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughChangesSheet interaction controls
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BottomSheetOrSheet
participant useEscKeyDown
participant Callback
User->>BottomSheetOrSheet: press Escape
BottomSheetOrSheet->>useEscKeyDown: process opened and closeOnEsc
useEscKeyDown->>Callback: invoke onEscKeyDown or onClose
User->>BottomSheetOrSheet: click overlay
BottomSheetOrSheet->>Callback: invoke onOverlayClick or onClose
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plasma-new-hope/src/hooks/useEscKeyDown.ts`:
- Around line 31-37: The useEffect listener registration in useEscKeyDown must
ensure Escape is handled only by the active, topmost sheet. Replace the
per-instance window listener behavior with an active-sheet stack or equivalent
topmost-sheet coordination, so nested sheets do not both invoke onKeyDown for
one key press while preserving cleanup when a sheet closes.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a63db746-ab74-4660-b75e-be66e77a91ad
📒 Files selected for processing (9)
packages/plasma-new-hope/src/components/BottomSheet/BottomSheet.tsxpackages/plasma-new-hope/src/components/BottomSheet/BottomSheet.types.tspackages/plasma-new-hope/src/components/Sheet/Sheet.tsxpackages/plasma-new-hope/src/components/Sheet/Sheet.types.tspackages/plasma-new-hope/src/hooks/index.tspackages/plasma-new-hope/src/hooks/useEscKeyDown.tsutils/api-tests/script.mjsutils/api-tests/src/components/BottomSheet/BottomSheet.api.test.tsxutils/api-tests/src/components/Sheet/Sheet.api.test.tsx
|
Theme Builder app deployed! https://plasma.sberdevices.ru/pr/plasma-theme-builder-pr-3021/ |
c43e1c8 to
59bd9e9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/plasma-new-hope/src/hooks/useEscKeyDown.ts`:
- Around line 18-24: Update the stack registration effect in useEscKeyDown so
every opened layer is registered regardless of closeOnEsc. Check whether the
layer is topmost before applying closeOnEsc, and only invoke onClose or
onEscKeyDown when that topmost layer allows Escape handling. Keep closeOnEsc in
callbackRef so prop updates do not remove and re-add the layer.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 51a180c3-5d96-4e6c-8423-134a0d209583
📒 Files selected for processing (1)
packages/plasma-new-hope/src/hooks/useEscKeyDown.ts
59bd9e9 to
06245d0
Compare
Core
BottomSheet, Sheet
What/why changed
Summary by CodeRabbit
New Features
Tests
📦 Published PR as canary version:
Canary Versions✨ Test out this PR locally via:
npm install @salutejs/plasma-asdk@0.387.0-canary.3021.31179775631.0 npm install @salutejs/plasma-b2c@1.629.0-canary.3021.31179775631.0 npm install @salutejs/plasma-colors@0.18.0-canary.3021.31179775631.0 npm install @salutejs/plasma-core@1.236.0-canary.3021.31179775631.0 npm install @salutejs/plasma-giga@0.356.0-canary.3021.31179775631.0 npm install @salutejs/plasma-homeds@0.356.0-canary.3021.31179775631.0 npm install @salutejs/plasma-hope@1.383.0-canary.3021.31179775631.0 npm install @salutejs/plasma-icons@1.245.0-canary.3021.31179775631.0 npm install @salutejs/plasma-new-hope@0.373.0-canary.3021.31179775631.0 npm install @salutejs/plasma-tokens@1.147.0-canary.3021.31179775631.0 npm install @salutejs/plasma-tokens-b2b@1.61.0-canary.3021.31179775631.0 npm install @salutejs/plasma-tokens-b2c@0.72.0-canary.3021.31179775631.0 npm install @salutejs/plasma-tokens-core@0.9.0-canary.3021.31179775631.0 npm install @salutejs/plasma-tokens-web@1.76.0-canary.3021.31179775631.0 npm install @salutejs/plasma-typo@0.49.0-canary.3021.31179775631.0 npm install @salutejs/plasma-web@1.631.0-canary.3021.31179775631.0 npm install @salutejs/sdds-bizcom@0.361.0-canary.3021.31179775631.0 npm install @salutejs/sdds-cs@0.365.0-canary.3021.31179775631.0 npm install @salutejs/sdds-dfa@0.359.0-canary.3021.31179775631.0 npm install @salutejs/sdds-finai@0.352.0-canary.3021.31179775631.0 npm install @salutejs/sdds-insol@0.356.0-canary.3021.31179775631.0 npm install @salutejs/sdds-insol-next@0.355.0-canary.3021.31179775631.0 npm install @salutejs/sdds-netology@0.360.0-canary.3021.31179775631.0 npm install @salutejs/sdds-os@0.31.0-canary.3021.31179775631.0 npm install @salutejs/sdds-platform-ai@0.360.0-canary.3021.31179775631.0 npm install @salutejs/sdds-sbcom@0.361.0-canary.3021.31179775631.0 npm install @salutejs/sdds-scan@0.359.0-canary.3021.31179775631.0 npm install @salutejs/sdds-serv@0.360.0-canary.3021.31179775631.0 npm install @salutejs/core-themes@0.37.0-canary.3021.31179775631.0 npm install @salutejs/plasma-themes@0.59.0-canary.3021.31179775631.0 npm install @salutejs/sdds-themes@0.74.0-canary.3021.31179775631.0 npm install @salutejs/sdds-api-tests@0.18.0-canary.3021.31179775631.0 npm install @salutejs/plasma-cy-utils@0.166.0-canary.3021.31179775631.0 npm install @salutejs/plasma-sb-utils@0.237.0-canary.3021.31179775631.0 npm install @salutejs/plasma-tokens-utils@0.57.0-canary.3021.31179775631.0 # or yarn add @salutejs/plasma-asdk@0.387.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-b2c@1.629.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-colors@0.18.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-core@1.236.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-giga@0.356.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-homeds@0.356.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-hope@1.383.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-icons@1.245.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-new-hope@0.373.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-tokens@1.147.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-tokens-b2b@1.61.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-tokens-b2c@0.72.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-tokens-core@0.9.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-tokens-web@1.76.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-typo@0.49.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-web@1.631.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-bizcom@0.361.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-cs@0.365.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-dfa@0.359.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-finai@0.352.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-insol@0.356.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-insol-next@0.355.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-netology@0.360.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-os@0.31.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-platform-ai@0.360.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-sbcom@0.361.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-scan@0.359.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-serv@0.360.0-canary.3021.31179775631.0 yarn add @salutejs/core-themes@0.37.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-themes@0.59.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-themes@0.74.0-canary.3021.31179775631.0 yarn add @salutejs/sdds-api-tests@0.18.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-cy-utils@0.166.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-sb-utils@0.237.0-canary.3021.31179775631.0 yarn add @salutejs/plasma-tokens-utils@0.57.0-canary.3021.31179775631.0