-
Notifications
You must be signed in to change notification settings - Fork 185
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 stroke #2204
waffle stroke #2204
Conversation
test/output/waffleHref.svg
Outdated
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.
no visual difference here
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.
this test and the other ones below have a small visible difference, but for the best
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.
this would be unchanged
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.
this test is the one exhibiting the bug (see description)
d3f851e
to
6f95f12
Compare
6f95f12
to
76a9826
Compare
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.
A thing I noticed is that these waffles have invisible slivers that extend the full width of the band. Related to #2132 I guess. Need more time to work on this but we probably want to simplify the polygon we’re using to render the waffle cells.
Yes, it explains the gradient we see at the top of the "before" image in the description. It would be nice to clean this up too. (Related: #2132) |
integrated in #2215 |
@@ -142,7 +142,7 @@ function waffleRender({render, ...options}) { | |||
.attr("transform", template`translate(${x0},${y0})`) | |||
.attr("d", (i) => `M${polygon[i].join("L")}Z`) | |||
.attr("fill", (i) => `url(#${patternId}-${i})`) | |||
.attr("stroke", this.stroke == null ? null : (i) => `url(#${patternId}-${i})`) | |||
.attr("stroke", this.stroke == null ? null : "none") |
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.
Just noting that this is visually equivalent to saying:
.attr("stroke", "none")
I.e., there’s never a stroke applied to the path element, it’s just that sometimes we have to suppress it when it would otherwise be inherited from the parent G element.
closes #2186
(to apply after #2203)