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

tweak horizon example #753

Merged
merged 2 commits into from
Feb 10, 2022
Merged

tweak horizon example #753

merged 2 commits into from
Feb 10, 2022

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Feb 10, 2022

It feels much more natural to me that the color scale would be defined in band index offset space (0 = the first offset, 1,400 = the second offset, and so on). The previous construction was dependent on the labels being in lexicographic order.

I also don’t grok why we can’t use the ordinal sequential color schemes as they are out of the box? Why do we need to recrop them using range: [1 / (bands + 1), 1 - 1 / (bands + 1)]? That feels like a complex thing to remember when you want to make a horizon chart.

@mbostock mbostock requested a review from Fil February 10, 2022 18:31
@Fil
Copy link
Contributor

Fil commented Feb 10, 2022

The idea behing range: [1 / (bands + 1), 1 - 1 / (bands + 1)] is that I wanted the color to reflect the value in the middle of the band.

Not super important when you make one chart like here. However if you have several charts with different number of bands, I felt it would be more consistent to have the first band of 3 have the same value as the second band of 9.

Also, you might want to mix a horizon mark with other marks—e.g., a few points that you'd want to highlight for some reason.

But again, these two use cases are not present in that example :)

@Fil Fil merged commit 232f45e into fil/clip-165 Feb 10, 2022
@Fil Fil deleted the mbostock/horizon branch February 10, 2022 19:52
Fil added a commit that referenced this pull request Feb 10, 2022
* clip: true clips the mark to the frame

closes #165

related: #181

* reindex clip id

* Update src/style.js

Co-authored-by: Mike Bostock <[email protected]>

* Update src/style.js

Co-authored-by: Mike Bostock <[email protected]>

* data source (the actual notebook is broken atm)

* clippath => clip

* reformat horizon plot; nicer heuristic for color

* naming consistency

* tweak horizon example (#753)

* tweak horizon example

* use offset, not index

* nicer tick labels

Co-authored-by: Mike Bostock <[email protected]>
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.

2 participants