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

Mobile responsive Gas Navigation and Resizing SVG gas graphs #1765

Merged
merged 3 commits into from
Jul 17, 2018

Conversation

Kempo
Copy link

@Kempo Kempo commented Jul 16, 2018

Description

This pull request allows the gas navigation bars to adjust according to screen width allowing for mobile compatibility and window resizing. Furthermore, gas navigation links have added padding/length so as to make clicking them a lot easier. The SVG graph for gas history resizes to a readable and non-bleedable width length on initial window startup.

NOTE: for some reason, after cloning the repo and running it, only the gas history graph with no data showed (only axes and titles). The gas graph was not available to see or for me to test. Therefore, the graphs I tested resize properly, but unfortunately, while I do make data refresh calls for the graphing lines, I haven't tested the graphs with data.

Checklist
  • linter status: 100% pass (ran npm run stylelint:fix and npm run eslint:fix)
  • changes don't break existing behavior
Affected core subsystem(s)

app/dashboard/templates/shared/gas_nav.html
app/retail/templates/gas_history.html
app/retail/templates/gas.html

Testing

I have tested this change running Safari.

BEFORE:
screen shot 2018-07-16 at 4 43 20 pm

AFTER (with different window widths):
screen shot 2018-07-16 at 4 13 58 pm
screen shot 2018-07-16 at 4 14 36 pm
screen shot 2018-07-16 at 4 14 54 pm

Refers/Fixes

Fixes: #1742

@codecov
Copy link

codecov bot commented Jul 16, 2018

Codecov Report

Merging #1765 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1765   +/-   ##
=======================================
  Coverage   28.86%   28.86%           
=======================================
  Files         128      128           
  Lines       10076    10076           
  Branches     1328     1328           
=======================================
  Hits         2908     2908           
  Misses       7061     7061           
  Partials      107      107

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2732749...1330519. Read the comment docs.

@owocki
Copy link
Contributor

owocki commented Jul 17, 2018

this seems to break the graphs for me (screenshot: http://bits.owocki.com/3W1U443E3I1A/Screen%20Shot%202018-07-16%20at%206.03.02%20PM.png)

i suggest running ./manage.py sync_gas_prices in order to get some data to test with

@Kempo
Copy link
Author

Kempo commented Jul 17, 2018

hey @owocki
That's weird. I can't seem to reproduce your error..
(screenshot: https://user-images.githubusercontent.com/1707464/42789645-473f0572-891b-11e8-9991-40746f3886e5.png)

Could you click on history:399 in console to display the code that's emitting the error? The line numbers that appear in console are different than what's actually in the file.
It could be possible that a commit or something didn't go through BUT, according to my Atom text editor I seemed to have committed and pushed everything with this pull request.

@Kempo
Copy link
Author

Kempo commented Jul 17, 2018

@owocki
according to my file actually:
screen shot 2018-07-16 at 5 21 18 pm

I'm guessing it's the .x function call, but I don't think I ever touched that. Also, that seems to be the same as what's in the master repo right now... that's odd.

@Kempo
Copy link
Author

Kempo commented Jul 17, 2018

@owocki wow. I had some careless errors in gas.html. But after a little tweak, here's a preview and I'll commit and add to this request soon:

screen shot 2018-07-16 at 5 54 56 pm

…uick edit to gas_history so hopefully no more errors
@owocki
Copy link
Contributor

owocki commented Jul 17, 2018

checking now

@owocki owocki merged commit 7e430f0 into gitcoinco:master Jul 17, 2018
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.

Bug - gas page is not mobile friendly.
2 participants