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

Add grid system #150

Merged
merged 1 commit into from
Oct 19, 2018
Merged

Add grid system #150

merged 1 commit into from
Oct 19, 2018

Conversation

MarcinKasprowicz
Copy link
Contributor

grid

Notes:

  • Click on the button changes grid between 1, 2, 3 columns per row.
  • Graph shrinking/expanding - I left this as it is, hiding graphs help with performance -> remaining graphs will is smoother.
  • Switching between graphs is enabled only when at least two functions are monitored.
  • For smaller screens (below 1280px) max grid is 2 columns (when user will change the size of the browser below this value it will not automatically resize from 3 to 2 - edge case, I didn't want to implement that - laziness 🐵)

closes #117

@MarcinKasprowicz MarcinKasprowicz requested a review from a team October 17, 2018 14:53
Copy link
Collaborator

@gomoripeti gomoripeti left a comment

Choose a reason for hiding this comment

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

LGTM

  • Graph shrinking/expanding - I wanted to complain about this (why would someone want to collaps just one graph in a row) but the performance consideration convinced me

  • maybe I would like a bit smaller icon better (and maybe the bg could be same as the magnifier-search in the navbar) but after I started playing with the css, I was convinced that this looks perfectly fine as is

  • maybe the buttons could have tooltips (I realise none of them have)

@gomoripeti gomoripeti merged commit 5fa65fc into release_2.0 Oct 19, 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.

None yet

2 participants