🎨 Palette: Add tooltips to message branch navigation#836
Conversation
This commit adds tooltips to the "Previous branch" and "Next branch" buttons in the MessageBranch component. This improves discoverability for these icon-only buttons and provides clearer guidance for users navigating between different AI response versions. - Wrapped MessageBranchPrevious in Tooltip - Wrapped MessageBranchNext in Tooltip - Added unit tests to verify correct rendering and prevent nested buttons Co-authored-by: cungminh2710 <8063319+cungminh2710@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Greptile SummaryThis PR adds
Confidence Score: 5/5Safe to merge — the change adds cosmetic tooltip wrappers with no impact on branch navigation logic. The tooltip wrapping follows the exact same render-prop pattern already established by MessageAction, touches no navigation logic, and includes tests that validate the structural output. No regressions are introduced. No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/hyperlocalise-web/src/components/ai-elements/message.tsx | Wraps MessageBranchPrevious and MessageBranchNext buttons in Tooltip/TooltipTrigger using the render-prop pattern, consistent with MessageAction's existing implementation. |
| apps/hyperlocalise-web/src/components/ai-elements/message.test.tsx | New unit tests using renderToStaticMarkup verify tooltip-trigger data-slot and aria-label attributes, and assert no nested buttons are produced by the render prop pattern. |
Sequence Diagram
sequenceDiagram
participant User
participant TooltipTrigger
participant Button
participant TooltipContent
User->>TooltipTrigger: mouseenter / focus
TooltipTrigger->>Button: merged event handlers (render prop)
TooltipTrigger->>TooltipContent: "opens popup (side=bottom)"
TooltipContent-->>User: displays "Previous branch" / "Next branch"
User->>Button: click (when enabled)
Button->>Button: goToPrevious() / goToNext()
Reviews (2): Last reviewed commit: "🎨 Palette: Add tooltips to message bran..." | Re-trigger Greptile
| expect(markup).toContain('data-slot="tooltip-trigger"'); | ||
| expect(markup).toContain('aria-label="Previous branch"'); | ||
|
|
||
| // Ensure no nested buttons | ||
| const buttonCount = (markup.match(/<button/g) || []).length; | ||
| expect(buttonCount).toBe(1); |
There was a problem hiding this comment.
Tests don't verify tooltip text content
The tests check for structural attributes (data-slot, aria-label) but never assert that the tooltip content strings "Previous branch" / "Next branch" are actually present in the output. If someone accidentally swaps the tooltip text between the two components or changes the strings, these tests would still pass. Since TooltipContent renders via TooltipPrimitive.Portal, its content is emitted inline by renderToStaticMarkup, so a check like expect(markup).toContain('>Previous branch<') would work and would catch regressions in the tooltip label.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/hyperlocalise-web/src/components/ai-elements/message.test.tsx
Line: 17-22
Comment:
**Tests don't verify tooltip text content**
The tests check for structural attributes (`data-slot`, `aria-label`) but never assert that the tooltip content strings `"Previous branch"` / `"Next branch"` are actually present in the output. If someone accidentally swaps the tooltip text between the two components or changes the strings, these tests would still pass. Since `TooltipContent` renders via `TooltipPrimitive.Portal`, its content is emitted inline by `renderToStaticMarkup`, so a check like `expect(markup).toContain('>Previous branch<')` would work and would catch regressions in the tooltip label.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This commit adds tooltips to the "Previous branch" and "Next branch" buttons in the MessageBranch component. This improves discoverability for these icon-only buttons and provides clearer guidance for users navigating between different AI response versions. - Wrapped MessageBranchPrevious in Tooltip - Wrapped MessageBranchNext in Tooltip - Added unit tests to verify correct rendering and prevent nested buttons - Fixed formatting in test file to satisfy CI requirements Co-authored-by: cungminh2710 <8063319+cungminh2710@users.noreply.github.com>
Add tooltips to the message branch navigation buttons to improve accessibility and discoverability. Includes unit tests for the new functionality.
PR created automatically by Jules for task 13855804824030503634 started by @cungminh2710