Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
waffle tips #2132
waffle tips #2132
Changes from 8 commits
7dcccea
da7d094
1f0d634
caa5009
83d0a4d
f27e321
c7ef726
0233683
8ef4c82
57f8e3c
0c7224f
e927e69
4ef5034
1f4518c
28bdcaf
bed5d25
a67bd8d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
What is this change doing? It’s not covered by tests, and I don’t notice any difference in manual testing.
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.
Ah, so it’s removing the default behavior of the stack transform when the tip option is true, which is to use
pointer: "y"
on on waffleX, andpointer: "x"
on waffleY. I get why you’re doing it, but I’m not sure it’s an improvement — in a way it makes things worse by having lots of dead space where the tip isn’t activated even though you’re hovering over a waffle; you need to be within 40px of the waffle centroid for it to activate, but it isn’t always obvious where that centroid is, and why the tip isn’t activating.I wonder if this means we should set the maxRadius to Infinity for waffle marks (just so it’s clear that something is happening), or if it means the overall approach of choosing a single representative point for a mark is insufficient, and we need to supply the pointer transform with a full geometry so it can do e.g. a signed distance check.
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.
Perhaps another possibility is to change the pointer behavior when x (or y) is a band scale. But that’s a bit weird because the provided x here is already in screen space… (and the x scale may not even exist).
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.
I think setting maxRadius to infinity is the correct interim choice. But it would be neat to have a full geometry (both for geo and waffles) in the future.
Note that, for the geo mark, one thing that works well currently with a centroid-based pointer — and will be harder to solve with full geometry —, is to make "small countries" discoverable even when their shape is very small (islands are easy, the hard part is small countries surrounded by larger countries).
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.
Hmm, I’m not sure how we’d set maxRadius to Infinity for waffle marks. We can do it on the tip option for the derived tip, but that wouldn’t affect how the pointer transform behaves when applied to the waffle mark. 🤔 It kind of makes me think the waffle mark should supply a geometry channel, and then the pointer transform should compute the signed distance to the geometries… (That would also let us fix pointing at bars wider than 40px, too.)