Skip to content
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

Fix node highlight for all shapes #2430

Merged
merged 2 commits into from
Apr 11, 2017
Merged

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Apr 6, 2017

Fixes #2429.

Clouds

Instead of drawing one shape with both fill and stroke for the highlight, the same shape is rendered twice with slightly different radii, no fill and a very thick stroke which ensures uniform highlight thickness around the shape. It's a slightly hacky solution (especially with adjusting the colors & opacities), but it seems to work well. Thanks @foot for the idea!

cloud-fix2

Stacks

Instead of a stretched highlight shape, a regular highlight is drawn around the bottommost layer of the stack. On top of that, the whole stack is slightly skewed to the left for a more 3D feel - not sure if we want that change though, my initial motivation was to make the highlight look better, but now I think the whole stack looks better when skewed (even when not highlighted).

Turns out that using the border trick also made the stack highlight look better. Drawing an extra fake shape just for the highlight is still not ideal, but the alternative would be to use some heavy SVG masks/clipping which might give an even heaver load on rendering (and certainly be harder to implement).

stack-fixed2

@fbarl fbarl self-assigned this Apr 6, 2017
@fbarl fbarl requested a review from foot April 7, 2017 15:27
@fbarl fbarl force-pushed the fix-node-shape-highlight-path branch from 966bce2 to aedd48a Compare April 7, 2017 15:28
Copy link
Contributor

@foot foot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@fbarl fbarl merged commit 7c22c97 into master Apr 11, 2017
@fbarl fbarl deleted the fix-node-shape-highlight-path branch June 16, 2017 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Node highlight is off with some shapes
2 participants