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

Responsive graph in capture example #149

Merged
merged 3 commits into from
Dec 11, 2019
Merged

Responsive graph in capture example #149

merged 3 commits into from
Dec 11, 2019

Conversation

symt
Copy link
Contributor

@symt symt commented Dec 10, 2019

In response to #110 (for gci).

Make sure these boxes are checked before your pull request is ready to be reviewed and merged. Thanks!

  • tests pass -- see README.md for how to run them
  • code is in uniquely-named feature branch, and has been rebased on top of latest master (especially if you've been asked to make additional changes)
  • pull request are descriptively named
  • if possible, multiple commits squashed if they're smaller changes
  • reviewed/confirmed/tested by another contributor or maintainer

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/wiki/contributing-to-public-lab-software

Please alert developers on plots-dev@googlegroups.com when your request is ready or if you need assistance.

Thanks!


What it looks like: https://imgur.com/a/l8zFo9M
I'm unsure if that is the correct "item" to be looking at for the change. If it isn't, please tell me and I'll modify the query to fit the correct standard.

@welcome
Copy link

welcome bot commented Dec 10, 2019

Thanks for opening this pull request! Dangerbot will test out your code and reply in a bit with some pointers and requests.
There may be some errors, but don't worry! We're here to help! 👍🎉😄

@chen-robert
Copy link

Welcome to Public Labs @symt!

If you haven't already, be sure to check out the welcoming post at publiclab/plots2#6808 😄 You can also come to chat with us on gitter!

The changes look great, should be ready to merge now!
cc: @sidntrivedi012 @jywarren

Glad to have you here 🎉

@@ -42,3 +42,9 @@ hr {
height: 200px;
margin: 10px 0px;
}

@media only screen and (max-width: 1300px) {
Copy link
Member

Choose a reason for hiding this comment

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

Hi, @symt Thanks for opening the PR. Things are great so far but you have to add some changes. You have to set different widths for different standard screen sizes. And add gifs for it. Please so open the developer console in the gifs to make sure things are working fine. You can screen sizes for different ranges. Like for up to max-width: 480px, max-width: 992px and max-width: 1382px

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is something like this what you're looking for? I'm not sure if the CORS error is relevant here.

Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use Bootstrap layout classes to manage this in a standarized way?

https://getbootstrap.com/docs/4.3/layout/grid/

Copy link
Member

Choose a reason for hiding this comment

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

Hi, @symt changes look great. Can you add gifs showing different screen sizes on the browser?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@starkblaze01 starkblaze01 left a comment

Choose a reason for hiding this comment

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

LGTM

@jywarren jywarren merged commit 572d1d0 into publiclab:main Dec 11, 2019
@welcome
Copy link

welcome bot commented Dec 11, 2019

Congrats on merging your first pull request! 🙌🎉⚡️
Your code will likely be published to https://spectralworkbench.org in the next few days.
In the meantime, can you tell us your Twitter handle so we can thank you properly?
Now that you've completed this, you can help someone else take their first step!
See: Public Lab's coding community!

@jywarren
Copy link
Member

Super! Great work!!! Thank you!

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.

4 participants