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

media query for buttons #583

Merged
merged 4 commits into from Jan 6, 2019
Merged

media query for buttons #583

merged 4 commits into from Jan 6, 2019

Conversation

vibhorgupta-gh
Copy link

@vibhorgupta-gh vibhorgupta-gh commented Dec 31, 2018

Fixes #582

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

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

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

Thanks!

@vibhorgupta-gh
Copy link
Author

Fixed buttons. Introduced margin and proper width. @jywarren @tech4GT please have a look

screen shot 2018-12-31 at 11 02 05 pm

screen shot 2018-12-31 at 11 02 16 pm

@harshkhandeparkar
Copy link
Member

@VibhorCodecianGupta what about the quick buttons? They too overflow. Can you fix those? Its really annoying on smaller screens.

@harshkhandeparkar
Copy link
Member

How about giving the preview images a bootstrap class of .img?

@vibhorgupta-gh
Copy link
Author

@harshkhandeparkar will look into it!

@harshkhandeparkar
Copy link
Member

Sorry I think the class is .img-fluid which makes the image responsive.

@vibhorgupta-gh
Copy link
Author

vibhorgupta-gh commented Jan 4, 2019

@jywarren yes, this can be achieved using btn-block as well, but the white panel behind the two buttons doesn't adjust it's size, and additional css might be required there (which I haven't figured out yet). Example:

screen shot 2019-01-03 at 11 22 20 am

How would you suggest we proceed? Using btn-block gets the desired result without changing the size of the background panel though. This PR takes care of the panel too, which looks prettier IMO. Thoughts?

@harshkhandeparkar
Copy link
Member

How about arranging the buttons one below the other on small screens?

@harshkhandeparkar
Copy link
Member

We can do that with bootstrap d-sm-block but this is for bootstrap 4.x I don't know how to do this using bootstrap 3.x

@vibhorgupta-gh
Copy link
Author

Wouldn't that break the UX? I feel that would make the view very clogged and full of buttons, if the buttons are listed one below the other, because we already have 4 buttons on the screen

@harshkhandeparkar
Copy link
Member

I mean only the save sequence and gif buttons.

@harshkhandeparkar
Copy link
Member

The quick buttons are the biggest problem actually.

@vibhorgupta-gh
Copy link
Author

Yes, I get you. We could do that. Let's see what the maintainers have in mind!
JBTW, are you a maintainer of image-sequencer as well? 😅

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented Jan 4, 2019

I'm not. LOL I'm actually a GCI student. I met publiclab during GCI this year. 😅

@vibhorgupta-gh
Copy link
Author

Oh! Hahaha, that's great. Nice to see you stuck with the Org!

@harshkhandeparkar
Copy link
Member

Yep JS is my favourite programming language so I like publiclab/image-sequencer.

@jywarren
Copy link
Member

jywarren commented Jan 4, 2019 via email

@vibhorgupta-gh
Copy link
Author

@jywarren so the short-term solution according to you would be? I think adding btn-block for now should do the trick, we could look into custom media query for smaller screens in the long run. What say?

@jywarren
Copy link
Member

jywarren commented Jan 4, 2019 via email

@vibhorgupta-gh
Copy link
Author

Nice. I'll push the changes in a minute then.

@vibhorgupta-gh
Copy link
Author

@jywarren have a look if you may, pushed the easy fix. Looks like so:

Tablet view
screen shot 2019-01-04 at 11 19 52 pm

Mobile view
screen shot 2019-01-04 at 11 20 01 pm

@jywarren
Copy link
Member

jywarren commented Jan 4, 2019 via email

@vibhorgupta-gh
Copy link
Author

@jywarren scrolling works for the quick icons. Had to write a media query for the mobile screens because of the brightness icon not aligning with the left border of the panel by default like so:

screen shot 2019-01-05 at 1 43 39 pm

after putting the scroll and the media query to justify evenly (for mobile screens only, because it works perfectly otherwise), here is how it looks like (notice the scrollbar :D) :

screen shot 2019-01-05 at 1 45 13 pm

@jywarren
Copy link
Member

jywarren commented Jan 5, 2019

This looks great. It says I can't merge automatically, so I think with a rebase it can be merged. Thanks!

Also we'll figure out the compiling issues real soon - no worries.

@ghost ghost added the in-progress label Jan 6, 2019
@jywarren
Copy link
Member

jywarren commented Jan 6, 2019

Rebased and rebuilt!

@jywarren jywarren merged commit a522e13 into publiclab:main Jan 6, 2019
@jywarren
Copy link
Member

jywarren commented Jan 6, 2019

Done. Nice!!!

@ghost ghost removed the in-progress label Jan 6, 2019
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

3 participants