feat(blade-svelte): add showDragHandle prop to BottomSheet to optionally hide drag handle - #3902
Merged
rohankokane-dev merged 2 commits intoAug 26, 2026
Merged
Conversation
Adds a `showDragHandle` boolean prop (default true) to the Svelte BottomSheet so consumers can hide the drag handle pill. Adds a storybook control + "Without Drag Handle" story and a changeset. Co-authored-by: Cursor <cursoragent@cursor.com>
🦋 Changeset detectedLatest commit: 87692e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
(Review Cancelled - Superseded by a new run) |
Collaborator
🛡️ Coverage ReportSummaryFull Coverage Details |
![rzp-slash[bot]](https://avatars.githubusercontent.com/u/7713209?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✨ Agentic PR Review ✨
UI Review
✅ 4 passed
Passing checks (4)
| Check | Screenshot |
|---|---|
| ✅ Without Drag Handle story - drag handle hidden |  |
| ✅ Default story - drag handle visible |  |
| ✅ Storybook showDragHandle control toggle |  |
| ✅ Backdrop dismissal with drag handle hidden |  |
Usage
<script>
import { BottomSheet, BottomSheetHeader, BottomSheetBody } from '@razorpay/blade-svelte/components';
let isOpen = $state(false);
</script>
<BottomSheet isOpen={isOpen} onDismiss={() => isOpen = false} showDragHandle={false}>
{#snippet children()}
<BottomSheetHeader title="My Sheet" />
<BottomSheetBody>
{#snippet children()}
Content without drag handle
{/snippet}
</BottomSheetBody>
{/snippet}
</BottomSheet>| /** | ||
| * Toggles the drag handle (the pill affordance rendered at the top of the | ||
| * sheet). Set to `false` to hide it. Since the handle is the drag surface, | ||
| * hiding it also removes the drag-to-move affordance; the sheet can still be |
Contributor
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 9/10
Problem: The JSDoc states 'Since the handle is the drag surface, hiding it also removes the drag-to-move affordance' — this is factually incorrect. The Svelte BottomSheet wires a second DragGesture to the body's scroll element (BottomSheet.svelte lines 449-462) that is completely independent of showDragHandle. When showDragHandle={false}, users can still drag the sheet to move/dismiss it via the content area. A developer relying on this doc might set showDragHandle={false} expecting drag to be fully disabled, only to find content-area dragging still active.
Suggestion: Rewrite the JSDoc to accurately reflect that only the grab-handle drag surface is removed, while content-body drag-to-dismiss remains active. For example: 'Set to false to hide it. This removes the grab-handle drag surface; the sheet can still be dragged via the content body (if present), dismissed via the backdrop, esc, or programmatically.'
|
|
||
| feat(blade-svelte): add `showDragHandle` prop to BottomSheet to optionally hide the drag handle | ||
|
|
||
| `BottomSheet` now accepts a `showDragHandle` boolean prop (default `true`). Set it to `false` to hide the drag handle (the pill affordance at the top of the sheet). Since the handle is the drag surface, hiding it also removes the drag-to-move affordance; the sheet can still be dismissed via the backdrop, `esc`, or programmatically. |
Contributor
There was a problem hiding this comment.
🟠 [MAJOR] · code-quality-critique · confidence: 9/10
Problem: The changeset repeats the same inaccurate claim: 'Since the handle is the drag surface, hiding it also removes the drag-to-move affordance.' The content body DragGesture (BottomSheet.svelte:449-462) still allows drag-to-move/dismiss when the grab handle is hidden, so the drag-to-move affordance is NOT fully removed.
Suggestion: Correct the changeset to state that only the grab-handle drag surface is removed, while content-body drag remains available.
| * | ||
| * @default true | ||
| */ | ||
| showDragHandle?: boolean; |
Contributor
There was a problem hiding this comment.
🙏🏻 [NEEDS CLARIFICATION] · api-decision-critique · confidence: 9/10
The PR claims cross-framework parity with React PR #3896, but the React BottomSheet (packages/blade/src/components/BottomSheet/types.ts) does not currently have a showDragHandle prop. Has React PR #3896 been merged? If not, is the prop name 'showDragHandle' finalized in the React PR? The React internal component is named 'BottomSheetGrabHandle' — will the React prop also use 'showDragHandle' rather than 'showGrabHandle'?
| zIndex = BOTTOM_SHEET_Z_INDEX, | ||
| portalTarget, | ||
| testID, | ||
| showDragHandle = true, |
Contributor
There was a problem hiding this comment.
🔵 [MINOR] · api-decision-critique · confidence: 7/10
Problem: Prop ordering inconsistency: in types.ts, showDragHandle is placed between portalTarget and testID, but in the component destructuring it is placed after testID and before ...rest. This makes it harder to visually cross-reference the type definition against the implementation.
Suggestion: Move showDragHandle before testID in the destructuring to match the ordering in types.ts.
|
|
||
| /* Default story state (one bucket per story to keep them independent). */ | ||
| let isDefaultOpen = $state(false); | ||
| let isDragHandleOpen = $state(false); |
Contributor
There was a problem hiding this comment.
🔵 [MINOR] · api-decision-critique · confidence: 8/10
Problem: The state variable 'isDragHandleOpen' is misleadingly named. It controls whether the BottomSheet in the 'Without Drag Handle' story is open — not whether the drag handle is visible. A reader might confuse it with a toggle for drag handle visibility.
Suggestion: Rename to 'isWithoutDragHandleOpen' to match the pattern of other story state variables like isHeaderFooterOpen, isSingleSelectOpen, etc.
| {/snippet} | ||
| </Story> | ||
|
|
||
| <!-- Story: Without Drag Handle — hides the drag handle pill via showDragHandle={false}. --> |
Contributor
There was a problem hiding this comment.
🔵 [MINOR] · code-quality-critique · confidence: 6/10
Problem: The new story comment says '<!-- Story: Without Drag Handle ...' without a number, but it is inserted between 'Story 1: Default' and 'Story 2: With Header Footer'. This breaks the existing sequential numbering convention used by all other story comments in the file.
Suggestion: Either number it as 'Story 2' and renumber subsequent stories, or use a descriptive label without 'Story' to avoid confusion with the numbered series.
rzp-slash
Bot
added
the
Human Help Needed 🧑🏻💻
Contributor
|
🤖 Slash AI Review has been triggered. View execution logs |
Contributor
There was a problem hiding this comment.
✨ Agentic PR Review ✨
Status: Approved ✅
UI Review
✅ 6 passed
Passing checks (6)
Usage
import { BottomSheet } from '@razorpay/blade-svelte/components';
<BottomSheet isOpen={true} onDismiss={() => {}} showDragHandle={false}>...</BottomSheet>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports the React
showDragHandleAPI to the SvelteBottomSheet(@razorpay/blade-svelte). Consumers can now hide the drag handle (the pill affordance at the top of the sheet, internally the "grab handle").showDragHandle?: boolean— defaulttrue(backwards compatible, handle shown as before).<div>is wrapped in{#if showDragHandle}; the drag/measure$effects already guard ongrabHandleEl, so they no-op cleanly when it isn't rendered.showDragHandleboolean control (Playground) + a dedicatedWithout Drag Handlestory.minor).API decision
Mirrors the React PR (#3896) for cross-framework parity: positive
show*prefix,@default true, "drag handle" as the public term while internalGrabHandlenaming stays an implementation detail.Behavior notes
esc, or programmatically.Test plan
showDragHandlecontrol appears in Storybook and toggles the handleWithout Drag Handlestory renders the sheet without the pillyarn svelte-checkpasses (0 errors)Made with Cursor