-
Notifications
You must be signed in to change notification settings - Fork 9
Implement tooltip on custom call type #182
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
base: main
Are you sure you want to change the base?
Changes from all commits
1f1deda
efaa8ab
8c096be
965bfdc
6954551
2393641
9b01a6a
52c78eb
a2d597a
dd64e46
daf11e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@serverlessworkflow/diagram-editor": patch | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be minor as well and please update the file name |
||
| --- | ||
|
|
||
| Implementation of tooltip | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,6 +25,10 @@ | |||||||||||||
| */ | ||||||||||||||
|
|
||||||||||||||
| .dec-root { | ||||||||||||||
| --background: hsl(0 0% 100%); | ||||||||||||||
|
|
||||||||||||||
| --foreground: hsl(222.2 84% 4.9%); | ||||||||||||||
|
Comment on lines
+28
to
+30
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please use more specific names for these new variables, following the code style already here?
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| --sidebar: hsl(0 0% 98%); | ||||||||||||||
|
|
||||||||||||||
| --sidebar-foreground: hsl(240 5.3% 26.1%); | ||||||||||||||
|
|
@@ -43,6 +47,10 @@ | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| .dec-root.dark { | ||||||||||||||
| --background: hsl(222.2 84% 4.9%); | ||||||||||||||
|
|
||||||||||||||
| --foreground: hsl(210 40% 98%); | ||||||||||||||
|
Comment on lines
+50
to
+52
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| --sidebar: #2d3748; | ||||||||||||||
|
|
||||||||||||||
| --sidebar-foreground: hsl(240 4.8% 95.9%); | ||||||||||||||
|
|
@@ -61,6 +69,10 @@ | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| @theme inline { | ||||||||||||||
| --color-background: var(--background); | ||||||||||||||
|
|
||||||||||||||
| --color-foreground: var(--foreground); | ||||||||||||||
|
Comment on lines
+72
to
+74
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
|
|
||||||||||||||
| --color-sidebar: var(--sidebar); | ||||||||||||||
|
|
||||||||||||||
| --color-sidebar-foreground: var(--sidebar-foreground); | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,20 +47,18 @@ function TooltipContent({ | |
| ...props | ||
| }: React.ComponentProps<typeof TooltipPrimitive.Content>) { | ||
| return ( | ||
| <TooltipPrimitive.Portal> | ||
| <TooltipPrimitive.Content | ||
| data-slot="tooltip-content" | ||
| sideOffset={sideOffset} | ||
| className={cn( | ||
| "dec:z-50 dec:w-fit dec:origin-(--radix-tooltip-content-transform-origin) dec:animate-in dec:rounded-md dec:bg-foreground dec:px-3 dec:py-1.5 dec:text-xs dec:text-balance dec:text-background dec:fade-in-0 dec:zoom-in-95 dec:data-[side=bottom]:slide-in-from-top-2 dec:data-[side=left]:slide-in-from-right-2 dec:data-[side=right]:slide-in-from-left-2 dec:data-[side=top]:slide-in-from-bottom-2 dec:data-[state=closed]:animate-out dec:data-[state=closed]:fade-out-0 dec:data-[state=closed]:zoom-out-95", | ||
| className, | ||
| )} | ||
| {...props} | ||
| > | ||
| {children} | ||
| <TooltipPrimitive.Arrow className="dec:z-50 dec:size-2.5 dec:translate-y-[calc(-50%_-_2px)] dec:rotate-45 dec:rounded-[2px] dec:bg-foreground dec:fill-foreground" /> | ||
| </TooltipPrimitive.Content> | ||
| </TooltipPrimitive.Portal> | ||
| <TooltipPrimitive.Content | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I saw in the official implementation they use
|
||
| data-slot="tooltip-content" | ||
| sideOffset={sideOffset} | ||
| className={cn( | ||
|
kumaradityaraj marked this conversation as resolved.
|
||
| "dec:z-50 dec:w-fit dec:origin-(--radix-tooltip-content-transform-origin) dec:animate-in dec:rounded-sm dec:bg-foreground dec:px-1 dec:py-1 dec:text-[9px] dec:text-balance dec:text-background dec:fade-in-0 dec:zoom-in-95 dec:data-[side=bottom]:slide-in-from-top-2 dec:data-[side=left]:slide-in-from-right-2 dec:data-[side=right]:slide-in-from-left-2 dec:data-[side=top]:slide-in-from-bottom-2 dec:data-[state=closed]:animate-out dec:data-[state=closed]:fade-out-0 dec:data-[state=closed]:zoom-out-95", | ||
| className, | ||
| )} | ||
| {...props} | ||
| > | ||
| {children} | ||
| <TooltipPrimitive.Arrow className="dec:z-50 dec:size-2.5 dec:translate-y-[calc(-50%_-_2px)] dec:rotate-45 dec:rounded-[2px] dec:bg-foreground dec:fill-foreground" /> | ||
| </TooltipPrimitive.Content> | ||
| ); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -30,6 +30,8 @@ import { | |||
| import { DEFAULT_NODE_SIZE } from "../../../src/react-flow/diagram/autoLayout"; | ||||
| import { en } from "../../../src/i18n/locales/en"; | ||||
| import { renderWithProviders } from "../../test-utils/render-helpers"; | ||||
| import { TooltipProvider } from "../../../src/components/ui/tooltip"; | ||||
| import userEvent from "@testing-library/user-event"; | ||||
|
|
||||
| function testNode( | ||||
| id: string, | ||||
|
|
@@ -256,19 +258,52 @@ describe("React Flow custom node types", () => { | |||
| }), | ||||
| ]; | ||||
| renderWithProviders( | ||||
| <div> | ||||
| <RF.ReactFlow | ||||
| nodeTypes={ReactFlowNodeTypes} | ||||
| nodes={nodesWithUnknownBadges} | ||||
| edges={allEdges} | ||||
| /> | ||||
| </div>, | ||||
| <TooltipProvider> | ||||
| <div> | ||||
| <RF.ReactFlow | ||||
| nodeTypes={ReactFlowNodeTypes} | ||||
| nodes={nodesWithUnknownBadges} | ||||
| edges={allEdges} | ||||
| /> | ||||
| </div> | ||||
| </TooltipProvider>, | ||||
| ); | ||||
|
|
||||
| const callBadge = screen.getByTestId("call-node-n1-badge-custom"); | ||||
| expect(callBadge).toBeInTheDocument(); | ||||
| expect(callBadge.textContent).toBe("customCall"); | ||||
| expect(callBadge).toHaveAttribute("title", "customCall"); | ||||
|
kumaradityaraj marked this conversation as resolved.
|
||||
| }); | ||||
|
kumaradityaraj marked this conversation as resolved.
|
||||
|
|
||||
| it("should render the raw value as a custom badge for an unknown subtype and display it in the tooltip", async () => { | ||||
| const user = userEvent.setup(); | ||||
|
|
||||
| const nodesWithUnknownBadges = [ | ||||
| testNode("n1", GraphNodeType.Call, 100, "CallNode", { | ||||
| call: "customCall", | ||||
| }), | ||||
| ]; | ||||
|
|
||||
| renderWithProviders( | ||||
| <TooltipProvider> | ||||
| <div> | ||||
| <RF.ReactFlow | ||||
| nodeTypes={ReactFlowNodeTypes} | ||||
| nodes={nodesWithUnknownBadges} | ||||
| edges={allEdges} | ||||
| /> | ||||
| </div> | ||||
| </TooltipProvider>, | ||||
| ); | ||||
|
|
||||
| const callBadge = screen.getByTestId("call-node-n1-badge-custom"); | ||||
|
|
||||
| expect(callBadge).toBeInTheDocument(); | ||||
| expect(callBadge).toHaveTextContent("customCall"); | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is already tested by the previous test
Suggested change
|
||||
|
|
||||
| await user.hover(callBadge); | ||||
|
|
||||
| const tooltip = await screen.findByRole("tooltip"); | ||||
| expect(tooltip).toHaveTextContent("customCall"); | ||||
| }); | ||||
|
|
||||
| it("should render while/compete badges on container nodes", () => { | ||||
|
|
||||

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.
Let's use minor
https://semver.org/#summary