Skip to content

Warn users when facet data doesn't match mark data #1014

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

Merged
merged 13 commits into from
Jul 28, 2022

Conversation

mkfreeman
Copy link
Contributor

@mkfreeman mkfreeman commented Jul 27, 2022

Closes #1009. Provides a warning when facet data doesn't strictly match mark data.

Note, I'm unsure if the logic or wording are exactly what we want.

Logic to trigger the warning:

if(
  facet?.data && 
  mark?.data  && 
  mark.data.length === facet.data.length && 
  mark.data !== facet.data && 
  facet.data.length > 1) {}

Warning text: Facet data must strictly equal mark data for appropriate faceting. Make sure you aren't filtering your facet data in the Plot command

@mkfreeman mkfreeman requested review from mbostock and Fil July 27, 2022 20:20
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

Thanks, Mike. I’m trying to guide you on how to express the logic—let me know if I can clarify anything by meeting/chatting directly.

Copy link
Contributor

@Fil Fil left a comment

Choose a reason for hiding this comment

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

Please change the test/example also, because in this case it is not easily fixable: first, you would have to specify facet: true on the three marks, and second you'd be disappointed by the result :)

Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

Please remove the extra facet_warning.svg snapshot, too?

Copy link
Contributor

@Fil Fil left a comment

Choose a reason for hiding this comment

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

The facet-warning example doesn't "work" in the sense that, if you add facet: true, you still get a funny-looking chart. We should have a better example, even if it's only used as a test plot.
Capture d’écran 2022-07-28 à 10 37 41

mkfreeman and others added 4 commits July 28, 2022 09:37
* logic & wording

* remove fcet_warning.svg

* An arguably better test/example plot.
@mkfreeman mkfreeman requested a review from mbostock July 28, 2022 18:32
Not sure how to remove the warning from the yarn test log—but this doesn't seem to be blocking
@mkfreeman mkfreeman merged commit 549f078 into main Jul 28, 2022
@mkfreeman mkfreeman deleted the mkfreeman/facet-warning branch July 28, 2022 21:04
@Fil
Copy link
Contributor

Fil commented Jul 28, 2022

A small issue with the wording is that it mentions facet: "auto" when (most probably) the user has not specified a facet option.

@mbostock
Copy link
Member

True, but that is the documented default. We could change this warning to only trigger when the facet option is not specified at all.

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.

Warn users when facet data doesn't match mark data
3 participants