Skip to content

Faceting title and empty data #262

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
Nov 19, 2021
Merged

Faceting title and empty data #262

merged 13 commits into from
Nov 19, 2021

Conversation

chowington
Copy link
Member

Fixes VEuPathDB/web-eda#681

Sorry, I got a little lost in the adjacent work that was done @bobular. Does this reflect what we've discussed surrounding the above ticket?

@chowington chowington requested a review from bobular November 16, 2021 22:21
Copy link
Member

@bobular bobular left a comment

Choose a reason for hiding this comment

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

Cool - looks great. Please could you just add the following to FacetedPlot (<Component ... >)

  showNoData={data == null}

Then we can test it out in the EDA (all merged now).
I have a feeling the dark gray will be a bit overpowering. But we can tweak that to our hearts' content later on.

@bobular bobular self-requested a review November 16, 2021 22:49
@bobular
Copy link
Member

bobular commented Nov 16, 2021

Oh wait, I found something not quite right.

I played with the Boxplot story controls, and added my own title

image

I'm pretty sure the title shouldn't be obscured - so more like this that I tweaked in dev tools:
image

@bobular
Copy link
Member

bobular commented Nov 16, 2021

Thanks for the FacetedPlot change.

Here's how it looks in the EDA. Possibly a bit in your face?
image

@bobular
Copy link
Member

bobular commented Nov 16, 2021

We can get rid of the "NO DATA:" prefix now too 😄

I'd suggest a light grey background at quite high opacity, and black text for "No data" (just noticed, I think lowercase 'd' fits the rest of the app's sentence case style)

@chowington
Copy link
Member Author

Thanks for the feedback @bobular! I'll work on those changes

@chowington
Copy link
Member Author

@bobular Check it out now!

@chowington
Copy link
Member Author

@bobular What was the conclusion of the EDA dev meeting on this? Any styling changes?

@bobular
Copy link
Member

bobular commented Nov 17, 2021

@bobular What was the conclusion of the EDA dev meeting on this? Any styling changes?

Hi - yes there were. Lighter grey background (and maybe even higher opacity to achieve the same level of obscuration, if that's a word), smaller "No data" font size and try white text for "No data". Thanks!

@bobular
Copy link
Member

bobular commented Nov 17, 2021

I just found an edge case with scatter plot that doesn't look so good.

Could we gray out the entire plot (like you did at first!), have the No data text in #e8e8e8 gray (this is 'EDA no data gray') and put the plot title on top of everything, in black?

image

@chowington
Copy link
Member Author

git revert ;)

@chowington
Copy link
Member Author

@bobular Did you envision a different color for the overlay background? Setting the text to EDA No Data Gray© makes the text hard to see against the current background color, however making the background darker makes it more prominent on the page, which seemed to be an issue in the meeting.

@bobular
Copy link
Member

bobular commented Nov 17, 2021

I played around with grays. It seems we can go really light and opaque and it looks OK.

          backgroundColor: '#f8f8f8f8',
      color: '#e8e8e8',

@bobular
Copy link
Member

bobular commented Nov 17, 2021

Danica would like some special formatting for the title of the "No data" facet (that's not to be confused with facets with no data!!)

So when title === 'No data' some subtly distinctive styling. Maybe including special no data gray #e8e8e8. Perhaps a border or background?

@chowington
Copy link
Member Author

Danica would like some special formatting for the title of the "No data" facet (that's not to be confused with facets with no data!!)

So when title === 'No data' some subtly distinctive styling. Maybe including special no data gray #e8e8e8. Perhaps a border or background?

Hmmm, now it seems like we're really getting into web-eda concerns that don't belong in web-components... This is along the same lines when I was worried about building the overlay into web-components when we're not sure the overlay will be useful for every library that uses it. That's why I first suggested having web-eda create the overlay itself if it wants one and not have web-components worry about it at all. Thoughts?

@bobular
Copy link
Member

bobular commented Nov 18, 2021

Hmmm, now it seems like we're really getting into web-eda concerns that don't belong in web-components... This is along the same lines when I was worried about building the overlay into web-components when we're not sure the overlay will be useful for every library that uses it. That's why I first suggested having web-eda create the overlay itself if it wants one and not have web-components worry about it at all. Thoughts?

Absolutely agree there is EDA-creep going on here. And @dmfalke made a similar suggestion here. We already have some instances of 'No data' checks in web-components - so this wouldn't be the first time. Two wrongs don't make a right but we do have to get something working by tomorrow! I suggest being pragmatic and we can make a technical-debt ticket as needed.

@chowington
Copy link
Member Author

@bobular I just pushed the following. Let me know what you think:

image

@bobular
Copy link
Member

bobular commented Nov 19, 2021

The "No missing data" is a nice addition - love the stripes subtly drawing attention to something different going on here - though the need for it might go away after VEuPathDB/web-eda#676 (not scheduled for b55).

What Danica envisaged was something to make the 'No data' facet (when there is data for it, as in the screenshot below - yeah this is confusing even for us!) look distinctive*. I'll have a quick play with some simple ideas and post some images
image

* in the same way that "No data" is styled differently in the legend for the overlay variable

Copy link
Member

@bobular bobular left a comment

Choose a reason for hiding this comment

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

Yeah I think just go with Italics to match the legend. There was a suggestion to use the No Data Gray ™️ but I think that might be confusing (people will go hunting around for gray maybe)

I'll push the tiny change after this.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants