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

centroid label overlay UX improvements #1141

Merged
merged 10 commits into from Feb 12, 2020

Conversation

seve
Copy link
Member

@seve seve commented Feb 4, 2020

Generally improves the UX of the label overlay in two ways:

  • Drops opacity of the graph on overlay display
  • Consistently truncates long labels

image


This PR implements the following changes:

  • In client/src/components/graph/overlays/graphOverlayLayer.js
    • add display state
    • add callback for changing the display state
    • check display state and change opacity accordingly
  • In client/src/components/graph/overlays/centroidLabels.js
    • Add truncation check and implementation
    • Check display change and call onDisplayChange() if necessary

Closes #1121, Closes #1136

@seve
Copy link
Member Author

seve commented Feb 4, 2020

@liaprins-czi @colinmegill @sidneymbell
Small thing Colin and I observed:

With the opacity change, the colorby swatches on the left sidebar no longer exactly reflect the graph colors. However, they are uniformly changed. Is this good as is, or should I reflect the color change on the swatches?

@seve
Copy link
Member Author

seve commented Feb 4, 2020

@colinmegill Let me know what you think of my implementation of checking display state, is it bad practice to require children to call onDisplayChange()?

@liaprins-czi
Copy link

liaprins-czi commented Feb 4, 2020 via email

@seve seve requested a review from sidneymbell February 5, 2020 22:02
Copy link

@liaprins-czi liaprins-czi left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@seve
Copy link
Member Author

seve commented Feb 5, 2020

Waiting for @sidneymbell's opinion on swatch color mirroring before merging

Copy link
Contributor

@sidneymbell sidneymbell left a comment

Choose a reason for hiding this comment

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

This looks great! I think the swatch color is fine, given the mouseover indicators in the sidebar <-> graph.

@seve seve merged commit 5340a5f into master Feb 12, 2020
@seve seve deleted the seve/enhancement-overlay-opacity-drop branch March 16, 2020 17:42
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.

Enhance readability of cell labels when shown on the graph drop opacity of graph when labels are displayed
4 participants