jlfwong / speedscope

🔬 A fast, interactive web-based viewer for performance profiles.
https://www.speedscope.app
MIT License
5.59k stars 246 forks source link

Increase tooltip hover offset to avoid being obscured by mouse #447

Open sparrowt opened 1 year ago

sparrowt commented 1 year ago

Fixes #444

For context: the default cursor on Windows 10 is 16px wide. Most OSes allow you to make it much bigger so there's not necessarily a 'right' value here, but it feels sensible to at least avoid obscuring it when using defaults.

Given most cursors display below the pixel at which the cursor is pointing, a solution which works in more cases would probably need to make a change from the current decision to // Place the tooltip below the cursor but for now I have avoided doing this (I'm not sure of the historical reasons) and instead done the minimal improvement.

jlfwong commented 11 months ago

Hey, thanks for this contribution! Sorry I've been dragging my feet on this.

While I agree this definitely fixes a real problem when the cursor overlaps the content of the hover tip (either because of discrepancies between OS cursor sizes or due to OS-level cursor size settings), this specific fix results in the hover-tip being awkwardly far away from the cursor when the cursor is the default size on macOS.

A half-assed solution here would be to increase the default size specifically for Windows to still have the tight alignment that you get for default cursor sizes on mac. The heuristic would be something roughly like "use the smallest size that prevents overlap when OS-default cursor sizes are used".

I tried the other thing you alluded to, placing the hover-tip above instead of below. I can see that it fixes this particular problem, but it IMO creates another one. From an information hierarchy perspective, I find it really distracting having it partially cover the thing that I'm pointing to. So I'm reluctant to do that either.

The combination of not being able to detect cursor size and wanting it to be below the cursor is pretty unfortunate and suggests that a proper fix is to use a different construct than the hover tip altogether.

For now, what do you think about the suggestion to do an OS-specific hack and "use the smallest size that prevents overlap when OS-default cursor sizes are used"?

sparrowt commented 10 months ago

For now, what do you think about the suggestion to do an OS-specific hack and "use the smallest size that prevents overlap when OS-default cursor sizes are used"?

Sure that's fine by me (though I'm not sure how to do it!) - also does DPI come into it? e.g. if this specifies things in pixels but actually the cursor takes up far more pixels when on a non-100% DPI setting in Windows.

I'd be interested to see how it looks on macOS - if the width of the cursor on macOS isn't too much less than 16 then perhaps we could reduce the 20px (proposed in this PR) down to 16 and at least it would no longer be obscured by default on Windows whilst not adding too bit a gap on macOS?