-
Notifications
You must be signed in to change notification settings - Fork 6
fix(ItemBase): auto tooltip by default #799
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: f960691 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 |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the ItemBase component by enabling auto tooltips by default when content overflows. The tooltip system is upgraded with a new function-as-child pattern for better control over non-focusable elements.
Key changes:
- Auto tooltip is now enabled by default (
tooltip={true}
) in ItemBase for overflow detection - New function-as-child pattern for TooltipTrigger allows proper tooltip support for any element
- Refactored tooltip logic into a reusable
useAutoTooltip
hook
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/components/content/ItemBase/ItemBase.tsx | Major refactor to enable auto tooltips by default and extract tooltip logic into useAutoTooltip hook |
src/components/overlays/Tooltip/TooltipTrigger.tsx | Added function-as-child pattern support and deprecated activeWrap prop |
src/components/overlays/Tooltip/TooltipProvider.tsx | Updated to support function-as-child pattern with SSR compatibility |
src/components/overlays/Tooltip/Tooltip.tsx | Changed default text preset from 't3' to 't4' |
src/components/overlays/Tooltip/index.ts | Added TooltipProvider export |
src/components/overlays/Tooltip/Tooltip.stories.tsx | Added stories for new function pattern and reduced test timeouts |
src/components/content/ItemBase/ItemBase.stories.tsx | Added story to demonstrate auto tooltip functionality |
src/components/content/ItemBase/ItemBase.docs.mdx | Updated documentation with comprehensive tooltip usage examples |
src/test/setup.ts | Added ResizeObserver mock for test environment |
src/components/overlays/Dialog/Dialog.tsx | Removed unused Button import |
.changeset/ten-llamas-do.md | Added changeset documenting the feature |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
const childrenArray = rawChildrenArray as unknown as [ | ||
ReactElement | string | TooltipTriggerFunction, | ||
ReactElement, | ||
]; | ||
let [trigger, tooltip] = childrenArray; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type casting through unknown
suggests a type safety issue. Consider using proper type guards or restructuring the type definitions to avoid unsafe casting.
const childrenArray = rawChildrenArray as unknown as [ | |
ReactElement | string | TooltipTriggerFunction, | |
ReactElement, | |
]; | |
let [trigger, tooltip] = childrenArray; | |
// Type guard for childrenArray | |
let trigger: ReactElement | string | TooltipTriggerFunction | undefined; | |
let tooltip: ReactElement | undefined; | |
if ( | |
Array.isArray(rawChildrenArray) && | |
rawChildrenArray.length === 2 && | |
( | |
typeof rawChildrenArray[0] === 'string' || | |
typeof rawChildrenArray[0] === 'function' || | |
(rawChildrenArray[0] && typeof rawChildrenArray[0] === 'object' && (rawChildrenArray[0] as any).$$typeof) | |
) && | |
(rawChildrenArray[1] && typeof rawChildrenArray[1] === 'object' && (rawChildrenArray[1] as any).$$typeof) | |
) { | |
trigger = rawChildrenArray[0] as ReactElement | string | TooltipTriggerFunction; | |
tooltip = rawChildrenArray[1] as ReactElement; | |
} else { | |
// Optionally, you can throw, warn, or handle gracefully | |
if (process.env.NODE_ENV === 'development') { | |
console.warn( | |
'CubeUIKit: TooltipTrigger expects children to be [trigger, tooltip], where trigger is a ReactElement, string, or function, and tooltip is a ReactElement.' | |
); | |
} | |
trigger = undefined; | |
tooltip = undefined; | |
} |
Copilot uses AI. Check for mistakes.
isSelected, | ||
hotkeys, | ||
tooltip, | ||
tooltip = true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the default value from undefined to true
is a breaking change that will affect existing components. This should be documented as a breaking change rather than a patch.
tooltip = true, | |
tooltip, |
Copilot uses AI. Check for mistakes.
📦 NPM canary releaseDeployed canary version 0.0.0-canary-45f600f. |
🧪 Storybook is successfully deployed!
|
🏋️ Size limit report
Click here if you want to find out what is changed in this build |
Note
ItemBase now shows auto overflow tooltips by default and Tooltip gains function-as-child trigger support, with refactors, docs, stories, and tests updated.
tooltip
totrue
with auto overflow detection (viaResizeObserver
).useAutoTooltip
logic andrenderWithTooltip
; simplified focus handling; refactor to directItemBaseElement
rendering.WithAutoTooltip
(play function ensures visibility). Docs: expanded tooltip section and adjusted headings.TooltipProvider
: supports element or function-as-child (SSR-safe), now exported fromindex
.TooltipTrigger
: accepts function trigger, deprecatesactiveWrap
, addstargetRef
, type guards, and ref handling; positioning unchanged.Tooltip
: style preset bumped tot4
.Tooltip.docs.mdx
.Button
import.ResizeObserver
in setup.*.plan.md
.Written by Cursor Bugbot for commit f960691. This will update automatically on new commits. Configure here.