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

Bar mobile #35

Merged
merged 13 commits into from
Apr 13, 2021
Merged

Bar mobile #35

merged 13 commits into from
Apr 13, 2021

Conversation

javan-pohl
Copy link
Collaborator

What does this PR do?

This pull request fixes issues related to the StackedBarPlot not displaying correctly on mobile devices, specifically that the bars become transparent and the x-axis date values start to overlap each other and become unreadable at mobile width display resolutions (tested at ~400px). Implemented fixes include changing the stroke (border on SVG shapes) in frameProps[style] to "none", changing the date format of the x-axis tick labels from July 1, 2020 to 7/1 and reducing their fontsize from 12 to 11, and changing various paddings/margins/widths in CSS .

Where should the reviewer start?

StackedBarPlot.js

How should this be manually tested?

View the 'tested' page on varying resolutions, from normal desktop width down to ~400px.

Any background context you want to provide?

No. I think I've covered everything

What are the relevant tickets?

#34

Questions:

Please let me know if there's anything I can do to improve workflow or the code.

Copy link
Collaborator

@joel-oe-lacey joel-oe-lacey left a comment

Choose a reason for hiding this comment

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

Thanks for adding those changes @javan-pohl, the colors are definitely a lot more visible on the Stacked Bar. Regarding the package-lock.json, this project uses Yarn, so that's why it was showing up as whole new file.

They recently changed the endpoint paths which requires a quick URL fix, I'm adding that in here, so I'll remove the package-lock.json while I'm at it. Also as I had changes I was putting in for the daily snapshot, I'll throw those in here to consolidate.

@joel-oe-lacey joel-oe-lacey merged commit 5e3b130 into master Apr 13, 2021
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