feat(blade-svelte): Tooltip component#3364
Open
rohankokane-dev wants to merge 5 commits intomasterfrom
Open
Conversation
Made-with: Cursor
🦋 Changeset detectedLatest commit: 86575d0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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
|
✅ PR title follows Conventional Commits specification. |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 86575d0:
|
Collaborator
🛡️ Coverage ReportSummaryFull Coverage Details |
rohankokane-dev
commented
May 5, 2026
| <p class={tooltipTitleClass}>{title}</p> | ||
| {/if} | ||
| <p class={tooltipContentClass}>{content}</p> | ||
| <svg |
Contributor
Author
There was a problem hiding this comment.
Bug: Arrow SVG viewBox height is wrong
The viewBox uses ARROW_WIDTH (14) for both width and height, but ARROW_HEIGHT is 8. This means the arrow SVG has a square 14x14 viewBox while the path only draws within a 14x8 area. The extra vertical space may cause subtle positioning misalignment because floating-ui's arrow middleware measures the actual SVG element dimensions.
Suggested change
| <svg | |
| width={ARROW_WIDTH} | |
| height={ARROW_HEIGHT} | |
| viewBox="0 0 {ARROW_WIDTH} {ARROW_HEIGHT}" |
Contributor
Author
There was a problem hiding this comment.
Fixed in 86575d0. Updated height and viewBox to use ARROW_HEIGHT (8) so the SVG element matches the actual path bounds; floating-ui's arrow middleware now measures the correct dimensions.
rohankokane-dev
commented
May 5, 2026
| let resolvedPlacement = $state<TooltipPlacement>(placement); | ||
| let floatingX = $state(0); | ||
| let floatingY = $state(0); | ||
| let arrowX = $state<number | null>(null); |
Contributor
Author
There was a problem hiding this comment.
Nit: tooltipId uses Math.random() -- not SSR-safe
If blade-svelte ever supports SSR, Math.random() will produce different IDs on server vs client, causing a hydration mismatch for the aria-describedby / id pair. Consider using a deterministic counter or Svelte's $props.id pattern if available.
Not urgent since this is client-only today, but worth noting for future-proofing.
Contributor
Author
There was a problem hiding this comment.
Tracking — not blocking for the current client-only blade-svelte. Will switch to a module-level counter (same pattern AccordionItem now uses in #3348) when SSR support lands.
rohankokane-dev
commented
May 5, 2026
| /* Mount the bubble as soon as it should open and keep it mounted until the | ||
| * close transition finishes. We toggle `data-state` so CSS handles the | ||
| * fade/slide; `isMounted` controls whether the DOM node exists at all. */ | ||
| $effect(() => { |
Contributor
Author
There was a problem hiding this comment.
Potential issue: handleBubbleTransitionEnd may never fire
If CSS transitions are disabled (e.g. prefers-reduced-motion: reduce), interrupted, or if the duration token --duration-quick resolves to 0s, the transitionend event will never fire and isMounted stays true forever. This means the portal DOM node leaks until the component is destroyed.
Consider adding a fallback timeout, or checking for reduced motion and setting isMounted = false directly in the $effect when isOpen becomes false:
$effect(() => {
if (!isOpen) {
dataState = 'closed';
// Fallback: if transition doesn't fire within a reasonable window, unmount
const fallback = setTimeout(() => { isMounted = false; }, 300);
return () => clearTimeout(fallback);
}
// ...existing open logic...
});
Contributor
Author
There was a problem hiding this comment.
Fixed in 86575d0. Added a 240ms fallback setTimeout in the close-state effect that unmounts the bubble if transitionend doesn't fire (prefers-reduced-motion, --duration-quick: 0s, or interrupted transitions). Cleared on re-open or when the listener wins the race so we don't double-unmount.
rohankokane-dev
commented
May 5, 2026
| let arrowX = $state<number | null>(null); | ||
| let arrowY = $state<number | null>(null); | ||
|
|
||
| const tooltipId = `blade-tooltip-${Math.random().toString(36).slice(2, 10)}`; |
Contributor
Author
There was a problem hiding this comment.
Perf nit: getFloatingPlacementParts called twice in isHorizontal
This derived value calls getFloatingPlacementParts(placement) twice -- once for the left check and once for right. Since it splits a string and allocates a tuple each time, consider extracting once:
const placementSide = $derived(getFloatingPlacementParts(placement)[0]);
const isHorizontal = $derived(placementSide === 'left' || placementSide === 'right');This also simplifies readability.
Contributor
Author
There was a problem hiding this comment.
Tracking — out of scope for this must-fix pass. Will extract getFloatingPlacementParts(placement) into a single $derived in a follow-up.
rohankokane-dev
commented
May 5, 2026
| ); | ||
|
|
||
| /* Rotate the base ▼ path so the apex points at the trigger; anchor with | ||
| * `[side]: 100%` so the base stays flush with the bubble edge on all sides. */ |
Contributor
Author
There was a problem hiding this comment.
Consider: metaAttrs is not reactive to testID changes
metaAttrs is computed once at component init (plain const, not $derived), so if testID is changed dynamically by the parent, the meta attribute on the portal will be stale. Other components in this codebase (Badge, Alert) also do this at init time, so it may be an intentional pattern -- but worth calling out since the tooltip portal lives in document.body and might be harder to debug when test selectors don't update.
Contributor
Author
There was a problem hiding this comment.
Acknowledged. Matches the existing pattern across blade-svelte components (Badge, Alert, etc.); will revisit holistically rather than per-component.
rohankokane-dev
commented
May 5, 2026
| ): PortalActionReturn { | ||
| let resolved = resolveTarget(target); | ||
| resolved.appendChild(node); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Portal action does not save the original parent
When update() is called with a new target, the node is moved but there is no record of where it originally was. The destroy() just removes the node from whatever parent it is in. This is fine for the current Tooltip use case (always targets document.body, never updates), but the action is exported as a general-purpose utility from @razorpay/blade-svelte/utils.
If any future consumer relies on destroy() to restore the node to its original position (the common expectation for portal actions), they will be surprised. Consider either:
- Saving the original parent and restoring on destroy
- Using a placeholder
Commentnode pattern (common in Svelte portal implementations) - Or at minimum, documenting that
destroy()removes the node entirely rather than restoring it
Contributor
Author
There was a problem hiding this comment.
Acknowledged. The action is currently used only by Tooltip (always portals to document.body, never updates) so the simpler 'remove on destroy' semantics work. Will revisit when a second consumer needs restore-to-original-parent behavior.
rohankokane-dev
commented
May 5, 2026
| */ | ||
| .bubble[data-state='closed'] { | ||
| opacity: 0; | ||
| } |
Contributor
Author
There was a problem hiding this comment.
CSS: dark mode border selector relies on ancestor [data-color-scheme]
The selector [data-color-scheme='dark'] .bubble requires .bubble to be a descendant of [data-color-scheme]. However, the tooltip bubble is portaled to document.body. This means the dark-mode border will only apply if <body> or an ancestor of <body> has data-color-scheme="dark".
Can you confirm that BladeProvider sets data-color-scheme on a common ancestor that remains above the portal target? If it sets it on a wrapper div inside body rather than on <html> or <body> itself, these dark-mode styles will never match.
Contributor
Author
There was a problem hiding this comment.
Fixed in 86575d0. Verified that blade-svelte sets body[data-theme='dark'] (preview.js) which is what @razorpay/blade-core/tokens/theme.css uses (:root body[data-theme='dark']). Updated both selectors in tooltip.module.css from [data-color-scheme='dark'] to body[data-theme='dark'] so the dark-mode bubble border + arrow stroke actually apply when the bubble is portaled to document.body.
rohankokane-dev
commented
May 5, 2026
| @@ -0,0 +1,49 @@ | |||
| /** | |||
| * Svelte action that moves a node into a target element (defaults to | |||
| * `document.body`) for the lifetime of the action, then restores it on destroy. | |||
Contributor
Author
There was a problem hiding this comment.
JSDoc says "restores it on destroy" but it does not
The doc comment at line 3 says the action "moves a node into a target element...then restores it on destroy." But destroy() calls removeChild() -- it removes the node from the DOM entirely, it does not restore it to its original parent. The doc should be updated to match the actual behavior.
Contributor
Author
There was a problem hiding this comment.
Tracking — out of scope for this must-fix pass. Will update the JSDoc (or implement actual restore semantics) in a follow-up.
rohankokane-dev
commented
May 5, 2026
Contributor
Author
rohankokane-dev
left a comment
rohankokane-dev
left a comment
There was a problem hiding this comment.
Tooltip Svelte 5 Migration -- Review Summary
Overall this is a well-structured migration. The component follows established blade-svelte patterns (CVA in blade-core, $props() destructuring, $derived/$effect usage, meta/analytics attrs). The floating-ui integration is solid, the arrow math is correctly ported, and the stories provide good coverage. A few items to address:
Must Fix
-
Arrow SVG viewBox bug (Tooltip.svelte L249):
heightandviewBoxboth useARROW_WIDTH(14) instead ofARROW_HEIGHT(8). This creates a 14x14 SVG when the path only occupies a 14x8 area, which will cause floating-ui's arrow middleware to miscalculate positioning since it measures the actual DOM element. -
transitionendmay never fire (Tooltip.svelte L121): Ifprefers-reduced-motion: reduceis active, or--duration-quickresolves to0s, thetransitionendevent never fires andisMountedstaystrue. The portal DOM node leaks. Add a fallback timeout. -
Dark mode CSS selector + portal (tooltip.module.css L84):
[data-color-scheme='dark'] .bubblerequires.bubbleto be a descendant of the scheme attribute. Since the bubble is portaled todocument.body, verify that BladeProvider setsdata-color-schemeon<html>or<body>-- not on a wrapper div below body.
Should Fix
-
Portal JSDoc is inaccurate (portal.ts L3): Says "restores it on destroy" but
destroy()removes the node from the DOM entirely. Update the doc or implement restore-to-original-parent behavior. -
Duplicate
getFloatingPlacementPartscalls (Tooltip.svelte L87): Called twice to check left/right. Extract to a single$derivedfor clarity and to avoid redundant string splitting.
Nits / Future Considerations
-
Math.random()fortooltipId(L84): Not SSR-safe. Fine for now since blade-svelte is client-only, but consider a counter-based approach for future-proofing. -
metaAttrsnot reactive (L197): Computed once at init, not via$derived. Matches other blade-svelte components, so likely intentional, but worth documenting. -
Portal action design (portal.ts L37): Exported as a general utility but doesn't preserve the original parent. Document the "remove on destroy" semantics or add placeholder-based restore logic.
What Looks Good
- Clean separation: blade-core owns CSS/CVA, blade-svelte owns the component logic
- Proper use of
$effectwith cleanup return forautoUpdateandrequestAnimationFrame TooltipInteractiveWrapperwith context-based nesting assertionaria-describedby+role="tooltip"accessibility pattern- Comprehensive Storybook coverage (placement grid, interactive wrapper, custom triggers)
- Good inline documentation of React parity decisions and TODO tracking for hardcoded tokens
- Arrow SVG `viewBox` and `height` were using `ARROW_WIDTH` (14) for both axes, producing a 14x14 square viewBox while the path only draws a 14x8 arrow. floating-ui's arrow middleware measures the actual SVG element, so this caused subtle positioning misalignment. Switched to `ARROW_HEIGHT` for both `height` and the viewBox y-extent. - Bubble unmount could leak the portal DOM node when CSS transitions were skipped (`prefers-reduced-motion: reduce`, `--duration-quick` resolving to `0s`, or interrupted transitions) because `transitionend` never fired. Added a 240ms fallback timeout that unmounts if the transition listener doesn't fire first. - Dark-mode CSS used `[data-color-scheme='dark']` but blade-core/blade- svelte both set `body[data-theme='dark']`. Selector swapped so dark- mode bubble border + arrow stroke now actually apply. Co-authored-by: Cursor <cursoragent@cursor.com>
saurabhdaware
approved these changes
May 7, 2026
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
4 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.
Description
Migrates the React Tooltip component to Svelte 5.
Changes
packages/blade-svelte/src/components/Tooltip/(Tooltip + TooltipInteractiveWrapper)packages/blade-core/src/styles/Tooltip/packages/blade-core/src/utils/getFloatingPlacementParts/(ported helper)packages/blade-svelte/src/utils/portal.ts(Svelte action for portals)@floating-ui/dom(^1.6.0) as a runtime dep onblade-sveltepackages/blade-svelte/src/components/index.tsNotes
<span class="tooltip-trigger-wrapper">(Svelte has nocloneElementanalog).aria-describedbypreserves a11y.useDelayGroupis deferred to a follow-up (no@floating-ui/domequivalent).box-shadowandbackdrop-filterare hardcoded to React's runtime values pending--elevation-low-raisedand--backdrop-blur-hightoken CSS vars in blade-core.Additional Information
Artifacts (in worktree):
.cursor/artifacts/Tooltip/discovery-report.md.cursor/artifacts/Tooltip/migration-plan.md.cursor/artifacts/Tooltip/verification-report.md.cursor/artifacts/Tooltip/screenshots/See the verification report for the full validation summary.
Component Checklist
Made with Cursor