labels for swatches#621
Conversation
mbostock
left a comment
mbostock
left a comment
There was a problem hiding this comment.
I think my preference is that we show the scale’s label by default, consistently across legend types, but that you can disable it by setting label: null. That way we don’t need to special-case the behavior for swatches.
Fil
force-pushed
the
fil/label-swatches
branch
from
14492bd to
2eafe34
Compare
Fil
force-pushed
the
fil/label-swatches
branch
from
2eafe34 to
7dc8d9b
Compare
|
rebased |
mbostock
force-pushed
the
fil/label-swatches
branch
from
7dc8d9b to
63b7013
Compare
| ? swatches.call(div => div.append("p") | ||
| .text(label) | ||
| .style("font-weight", "bold") | ||
| .style("width", "100%")) |
There was a problem hiding this comment.
The parent element here has flex because the code expects that it contains the swatches. We need to change the styles to match the structure—we shouldn’t be setting the width style.
| `); | ||
|
|
||
| const palette = label | ||
| ? swatches.call(div => div.append("p") |
There was a problem hiding this comment.
I think we probably don’t want to use a P element here, and instead should have a DIV with more specific margins as needed. Probably slightly less vertical spacing between the label and the swatches than in this current example.
|
I’d like to revert the unrelated formatting changes… |
|
I'll do that. The goal of the formatting changes was to produce an output with consistent alignement. |
|
rebased—I still need to figure out why some alignments in the tests seem wrong |
Fil
force-pushed
the
fil/label-swatches
branch
from
abf5fdb to
d495760
Compare
|
superseded by #1885 |
2 participants
Note: the scale’s label is not shown for swatches, since most of the time it is obvious from the context. But it can be nice to write Plot.legend(…, label:"Industry"):
closes #834