feat: anchor tooltip to mark #959

marshallpete · 2024-11-01T18:07:52Z

Add options and support for snapping tooltips to the associated mark and set the position.

Issue

fixes #200

API

Options {
...
anchor: 'cursor' | 'mark';
position: Postion | Position[];
}

type Position = 'top' | 'bottom' | 'left' | 'right' | 'top-left' | 'top-right' | 'bottom-left' | 'bottom-right';

Each of the position options match the available anchor options on the vega label transform.

Behavior

Setting the anchor = 'mark' will make it so that the tooltip snaps to the associated mark. For voronoi paths, it will use the point that the voronoi path is derived from.

The tooltip will be positioned based on the position option. If only one position is provided, it will attempt to place the tooltip in that location. If the tooltip would be partially out of the viewport at this position or if the cursor location would overlap the tooltip, then it defaults back to snapping to the cursor position.

If multiple positions are supplied in the position option, then the algo will try to place the tooltip in each position until it finds one that is in the viewport and does not overlap the cursor location. If none of the positions meet this criteria, it will default back to anchoring to the cursor position.

The offsetX and offsetY options are honored. If a tooltip is placed directly to the left or right, only the offsetX is applied. If the tooltip is placed directly above or below the mark, only the offsetY is applied. If placed on one of the corners, both offsets are applied, just like with the cursor position calculation.

Positioning Example

Screen.Recording.2024-11-01.at.12.03.34.PM.mov

…shallpete/vega-tooltip into issue200/anchorTooltipToMark

marshallpete · 2024-11-01T21:20:51Z

This PR adds position. It would be pretty easy to also take advantage of the position option when positioning relative to the cursor like this issue calls out.
Maybe a follow up PR?

domoritz

This is great. Thanks for the pull request. I have a few comments.

domoritz · 2024-11-03T16:09:56Z

package.json

@@ -52,6 +52,7 @@
    "start": "yarn build && concurrently --kill-others -n Server,Rollup 'yarn serve' 'rollup -c -w'",
    "pretest": "yarn build:style",
    "test": "jest",
+    "watch": "jest --watch",


You can already run yarn test --watch so I don't think we need another command. The main reason I want to not add this is since we have all the other Vega packages and I want to move towards consistency with fewer commands and setup requirements.

domoritz · 2024-11-03T16:12:08Z

src/Handler.ts

+    if (this.options.anchor === 'mark') {
+      position = calculatePositionRelativeToMark(handler, event, item, this.el.getBoundingClientRect(), this.options);
+    } else {
+      position = calculatePositionRelativeToCursor(
+        event,
+        this.el.getBoundingClientRect(),
+        this.options.offsetX,
+        this.options.offsetY,
+      );
+    }


could this become

const position = (this.options.anchor === 'mark' ? calculatePositionRelativeToMark : calculatePositionRelativeToCursor)(event, this.el.getBoundingClientRect(), this.options)

The two methods can't have the exact same arguments since calculatePositionRelativeToMark needs the handler and the item.

Unless we want to pass the handler and item into calculatePositionRelativeToCursor but never use them.

domoritz · 2024-11-03T16:13:04Z

src/defaults.ts

 };

-export type Options = Partial<typeof DEFAULT_OPTIONS>;


What was wrong with this?

There wasn't anything wrong previously but when adding anchor and position, the method didn't work well.

Originally the types were inferred from the default object. Since the types are inferred, any string default will be inferred as type string. This worked great for all the previous properties of Options.

anchor and position are both string unions. So there is a set list of acceptable strings that can be provided. If we let the types be inferred by the default object, then the type for these would be inferred as string instead of cursor | mark etc.

The advantage of these two using a string union is intelliSense and general type checking. If a consumer is using TS, then they will get an error if they set anchor = 'mouse'. And all consumer (TS of JS) should get intelliSense suggestions as long intelliSense is enabled.

This is why I flipped the order so we define the Options type first and then define the DEFAULT_OPTIONS and set it's type to Options.

You can add as const or as 'cursor' | 'mark to the object, I think, to make the type stricter. But I don't feel strongly about flipping this so no need to change.

domoritz · 2024-11-03T16:15:58Z

src/position.ts

@@ -6,7 +11,7 @@
 * @param offsetX Horizontal offset.
 * @param offsetY Vertical offset.
 */
-export function calculatePosition(
+export function calculatePositionRelativeToCursor(


Can this method also use tooltipIsInViewport?

domoritz

Thanks. Looks great. Just a few small comments.

domoritz · 2024-11-05T00:30:11Z

src/Handler.ts

+            this.options.offsetX,
+            this.options.offsetY,


Let's refactor this to just take this.options so it's the same as above and can be combined as I suggested earlier.

Yes, the would mean passing maybe unused arguments but it simplifies the call here.

Yeah I like this suggestion better as well.

Sorry I missed that in your ternary suggestion.

domoritz · 2024-11-05T00:31:31Z

src/defaults.ts

+   *
+   * Example: bottom left would offset the tooltip to the bottom left by the offset value in pixels.
+   */
+  // offset?: number;


domoritz · 2024-11-05T00:32:56Z

src/position.ts

+}
+
+// Checks if the mouse is within the tooltip area
+function mouseIsOnTooltip(


Let's write a unit test for this.

domoritz · 2024-11-05T00:38:00Z

Oh, one last thing. Can you add a checkbox to the example pages to switch between cursor and mark anchoring? Would be nice to testing.

marshallpete · 2024-11-05T17:28:42Z

Updates have been made and a radio toggle was added to change the anchor option.

Screen.Recording.2024-11-05.at.10.27.33.AM.mov

domoritz · 2024-11-05T18:49:20Z

thank you @marshallpete!

Marshall Peterson and others added 9 commits October 30, 2024 15:12

feat: enable positioning the tooltip relative to the hovered mark

425276c

refactor: clean up

e82a302

test: add coverage for calculatePositionRelativeToCursor()

9007b84

feat: added x and y offset for tooltips relative to hovered marks

c3a8348

chore: made diagonal positioning consistent for relative mark tooltips

c63c9c3

Merge branch 'issue200/anchorTooltipToMark' of https://github.com/mar…

18e4164

…shallpete/vega-tooltip into issue200/anchorTooltipToMark

test: add test coverage

6faccf2

chore: clean up

bb649db

chore: clean up

aca9ca6

marshallpete changed the title ~~Issue200/anchor tooltip to mark~~ feat: anchor tooltip to mark Nov 1, 2024

trigger build

aba5db5

domoritz requested changes Nov 3, 2024

View reviewed changes

Marshall Peterson added 3 commits November 4, 2024 10:22

build: remove unnecessary script

0b93c4c

chore: simplify statement

92219b8

refactor: use utility methods to validate tooltip position

f3adf41

domoritz approved these changes Nov 5, 2024

View reviewed changes

Marshall Peterson added 4 commits November 5, 2024 09:10

fix: remove commented code

eb18081

refactor: simplify arguments to calculatePositionRelativeToCursor()

ea323fa

test: add tests for position utilities

47c7ab1

feat: add options controls to example pages

62c97d1

Marshall Peterson added 2 commits November 5, 2024 11:22

fix: incorrect form value

c71a5a6

refactor: simplify code

96ad4da

domoritz merged commit 5a95192 into vega:main Nov 5, 2024
4 checks passed

joelostblom mentioned this pull request Nov 5, 2024

Add small caret/arrow to indicate which mark a tooltip belongs to #973

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: anchor tooltip to mark #959

feat: anchor tooltip to mark #959

marshallpete commented Nov 1, 2024 •

edited by domoritz

Loading

marshallpete commented Nov 1, 2024

domoritz left a comment

domoritz Nov 3, 2024

domoritz Nov 3, 2024

marshallpete Nov 4, 2024

marshallpete Nov 5, 2024

domoritz Nov 3, 2024

marshallpete Nov 4, 2024

domoritz Nov 5, 2024

domoritz Nov 3, 2024

domoritz left a comment

domoritz Nov 5, 2024

domoritz Nov 5, 2024

marshallpete Nov 5, 2024

marshallpete Nov 5, 2024

domoritz Nov 5, 2024

domoritz Nov 5, 2024

domoritz commented Nov 5, 2024

marshallpete commented Nov 5, 2024

domoritz commented Nov 5, 2024

feat: anchor tooltip to mark #959

feat: anchor tooltip to mark #959

Conversation

marshallpete commented Nov 1, 2024 • edited by domoritz Loading

Issue

API

Behavior

Positioning Example

marshallpete commented Nov 1, 2024

domoritz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domoritz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domoritz commented Nov 5, 2024

marshallpete commented Nov 5, 2024

domoritz commented Nov 5, 2024

marshallpete commented Nov 1, 2024 •

edited by domoritz

Loading