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

Reduce layout flicker on Region Details #652

Merged
merged 6 commits into from Nov 25, 2020
Merged

Reduce layout flicker on Region Details #652

merged 6 commits into from Nov 25, 2020

Conversation

dlaliberte
Copy link
Contributor

closes #637

Prerequisites:

  • Unless it is a hotfix it should be merged against the dev branch
  • Branch is up-to-date with the branch to be merged with, i.e. dev
  • Build is successful
  • Code is cleaned up and formatted

Summary

Change layout and formatting to keep legend columns stable as user hovers over various values.
Also fix legend color for first signal to match gray color in chart.

@netlify
Copy link

netlify bot commented Nov 24, 2020

Preview link ready!

Built with commit 6220832

https://deploy-preview-652--cmu-delphi-covidcast.netlify.app

@dlaliberte
Copy link
Contributor Author

There are a couple edge cases that still need a bit of work. e.g. With one column, the table expands too much if the window is wide.

@dlaliberte dlaliberte changed the title Dlaliberte/region Reduce layout flicker on Region Details Nov 24, 2020
@tildechris
Copy link
Contributor

Looks like there is still significant flicker:

E3o72DlvNF

I think we need to wait for the next release to take a closer look at this.

@tildechris
Copy link
Contributor

Previous screengrab was from the main page, here's one from the region details view:

IGvnwXmXBQ

@dlaliberte
Copy link
Contributor Author

Looks like there is still significant flicker:

Flickering tooltip is different from flickering layout of the charts and tables. The tooltip flickers less than it used to, but there are still some more cases when it is drawn off the window that it flickers.

I think we need to wait for the next release to take a closer look at this.

This change is about the flickering layout. It's pretty rock solid now. I wouldn't wait.

@tildechris
Copy link
Contributor

Ah, as per our discussion, you fixed the flicker with the chart area, and the tooltip flicker is a different issue. Let's move forward with the review.

Copy link
Contributor

@tildechris tildechris left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks Daniel, you can merge.

@dlaliberte dlaliberte merged commit 067bdb3 into dev Nov 25, 2020
@dlaliberte dlaliberte deleted the dlaliberte/region branch November 25, 2020 20:20
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.

None yet

2 participants