[WEB-4724] feat: tooltip component added to propel package#7613
[WEB-4724] feat: tooltip component added to propel package#7613sriramveeraghanta merged 4 commits intopreviewfrom
Conversation
WalkthroughAdds public exports for Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant TooltipModule as Tooltip
participant Base as BaseTooltip (Provider/Root/Trigger)
participant Portal as Portal
participant Pos as Positioner
participant Popup as Popup
App->>TooltipModule: Render Tooltip(props)
TooltipModule->>TooltipModule: compute finalSide/finalAlign (convertPlacementToSideAndAlign)
TooltipModule->>Base: instantiate Provider/Root (openDelay, closeDelay, disabled)
Base->>Base: render Trigger (children)
App-->>Base: user hover/focus
Base->>Portal: mount popup
Portal->>Pos: position using side/align/offset
Pos->>Popup: render heading (opt) and content
App-->>Base: leave/blur or disabled => unmount
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/propel/src/tooltip/root.tsx (4)
44-58: Tighten/align the public type surface; remove unused prop.
- Make
ITooltipPropsexportable for consumers.- Allow rich content in
tooltipHeading(currently string-only).renderByDefaultis unused; remove or implement.-type ITooltipProps = { - tooltipHeading?: string; - tooltipContent: string | React.ReactNode; +export type ITooltipProps = { + tooltipHeading?: React.ReactNode; + tooltipContent: React.ReactNode; position?: TPosition; children: React.ReactElement; disabled?: boolean; className?: string; openDelay?: number; closeDelay?: number; isMobile?: boolean; - renderByDefault?: boolean; side?: Side; align?: Align; sideOffset?: number; };Follow-up: if
renderByDefaultis intended, please clarify behavior (e.g., always render portal content for SEO/testing) and wire it into the component; otherwise, delete it to avoid API bloat.
22-24: ExportSideandAligntypes to help consumers with type-safety.Many consumers will want to type their props or constants consistently with the component API.
-type Side = "top" | "bottom" | "left" | "right"; -type Align = "start" | "center" | "end"; +export type Side = "top" | "bottom" | "left" | "right"; +export type Align = "start" | "center" | "end";
25-42: Clarify “auto” placement semantics or support true auto-placement.
"auto"/"auto-start"/"auto-end"currently map to fixed{ side: "bottom", align: ... }. That may surprise consumers expecting true dynamic placement (flip/shift) behavior.
- If Base UI supports collision-aware placement/flip, consider mapping
"auto"to that behavior instead of a fixed side.- At minimum, document that
"auto*"degrades to bottom-based placements for now.Would you like me to draft a README snippet documenting placement precedence and current
"auto"behavior?
61-66: Great: pure conversion helper invites unit tests.Consider adding a tiny test matrix to lock the mapping contract.
I can add a test file with cases for every TPosition and the default fallback if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/propel/package.json(2 hunks)packages/propel/src/tooltip/index.ts(1 hunks)packages/propel/src/tooltip/root.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Analyze (python)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
- GitHub Check: Build and lint web apps
🔇 Additional comments (3)
packages/propel/package.json (1)
21-22: New popover and tooltip exports verifiedBoth entry points were confirmed to exist at
- packages/propel/src/popover/index.ts
- packages/propel/src/tooltip/index.ts
These exports are wired correctly—approving the changes.
packages/propel/src/tooltip/root.tsx (1)
91-116: Confirm BaseTooltip API props and styling targetI couldn’t find any other usages of
BaseTooltipin the repo to cross-reference prop names, so please verify against the package’s TS definitions or docs that:
<BaseTooltip.Trigger render={children}>is correct and thatTriggerexpects arenderprop rather than children or a different API.<BaseTooltip.Positioner … render={<BaseTooltip.Popup/>}>is valid and thatPositioneraccepts bothclassNameandrenderhere (some libraries expect the styling onPopupinstead).- If
classNamebelongs onPopupinstead ofPositioner, move it accordingly to ensure your custom styles actually apply.Lastly, if you’re creating multiple tooltips under the same tree, consider hoisting a single
<BaseTooltip.Provider>to a higher level (e.g. around your app or a section) instead of wrapping each tooltip individually to avoid redundant context providers and potential performance overhead.packages/propel/src/tooltip/index.ts (1)
1-1: Public re-export looks good.This cleanly exposes
Tooltipand helpers from a stable entry point.
2 participants
Description
This PR adds the Tooltip component to the Propel UI package.
Summary by CodeRabbit