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

Rework restroom-list view to be mobile responsive #483

Merged
merged 1 commit into from
Jun 13, 2018

Conversation

tymc7
Copy link

@tymc7 tymc7 commented Jun 9, 2018

Context

Summary of Changes

  • Moves all .restroom-list code (including @media queries) to own file
  • Updated .restroom-list class to be a mobile first by changing all class updates to build on mobile rather than break down from mobile.
  • Fixed issue in _screen_sizes.scss to utilize variable listed
  • Updated #list to be a ul element to be more semantic
  • Dropped size of icons in search view because they were too large
  • Updated size of toilet icon to be more consistent across devices in the .restroom-list view

Checklist

  • Tested Mobile Responsiveness
  • Added Unit Tests (Wasn't sure if I should because it's all visual updates)
  • CI Passes
  • Deploys to Heroku on test Correctly (Maintainers will handle)
  • Added Documentation (Service and Code when required)

Screenshots

screen shot 2018-06-08 at 11 19 39 pm
screen shot 2018-06-08 at 11 19 44 pm
screen shot 2018-06-08 at 11 19 49 pm

Before

screen shot 2018-06-08 at 11 47 38 pm

After

screen shot 2018-06-08 at 11 19 39 pm

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jun 11, 2018

This looks really cool, thanks!

I'm going to get it loaded on my machine and try it out.

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jun 11, 2018

One little difference this makes (beside fixing #480) is that is can make the restroom icon rather small in some cases:

280-by-420-screenshot-Firefox-Windows-10

I think it looks fine, and it definitely helps make good use of the screen width without anything overlapping like it used to. I'm in favor, overall.

I love that this is fully responsive, and that the icon takes up just enough space now. And obviously the overlap issue is gone. Also, thank you for making the HTML more semantic by using <ul> and <li> together. 👍

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jun 11, 2018

Another minor change is that it makes margins around the form about 10 or 20 pixels wider on the "Submit a New Restroom" page, but again it looks fine to me.

submit-new-restroom-page-with-wider-margins

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jun 11, 2018

For reasons totally unrelated to this PR, the Travis CI run failed. (It was a network error between Travis and the Gem server, apparently.)

I'm re-starting the Travis CI build -- presumably it will pass this time.

Edit: It passed.

Copy link
Contributor

@DeeDeeG DeeDeeG left a comment

Choose a reason for hiding this comment

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

I approve.

Any other opinions, @mi-wood, @tkwidmer, everybody?

@DeeDeeG
Copy link
Contributor

DeeDeeG commented Jun 12, 2018

Will merge soon if there are no objections. 👍

@DeeDeeG DeeDeeG merged commit cf485df into RefugeRestrooms:develop Jun 13, 2018
@DeeDeeG DeeDeeG mentioned this pull request Oct 11, 2018
5 tasks
DeeDeeG pushed a commit to DeeDeeG/refugerestrooms that referenced this pull request Oct 15, 2018
DeeDeeG pushed a commit to DeeDeeG/refugerestrooms that referenced this pull request Nov 3, 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.

2 participants