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

fix mobile header #569

Merged
merged 3 commits into from
Nov 14, 2017
Merged

fix mobile header #569

merged 3 commits into from
Nov 14, 2017

Conversation

lislis
Copy link
Member

@lislis lislis commented Nov 7, 2017

part of #519

screenshot-2017-11-7 rorganize it find a rails girls study group 2

Logo, main-nav and profile-nav are now visible and usable on every screen size

@lislis lislis requested a review from sareg0 November 7, 2017 20:41
@@ -5,14 +5,13 @@
<title>RoRganize.it | Find a Rails Girls Study Group</title>
<%= stylesheet_link_tag (params[:redesign] ? 'redesign' : 'application'), media: "all" %>
<%= csrf_meta_tags %>
<link href='//fonts.googleapis.com/css?family=Pacifico' rel='stylesheet' type='text/css'>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@@ -5,14 +5,13 @@
<title>RoRganize.it | Find a Rails Girls Study Group</title>
<%= stylesheet_link_tag (params[:redesign] ? 'redesign' : 'application'), media: "all" %>
Copy link
Contributor

@sareg0 sareg0 Nov 8, 2017

Choose a reason for hiding this comment

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

Do you think we can also get rid of this? We can start a new issue for it, so as not to scope creep this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good catch! I can definitely remove that

@sareg0
Copy link
Contributor

sareg0 commented Nov 8, 2017

Thanks soooo much for this. Since there is a fair bit of re-organization of the files, could you point out the fixes that solve the particular UI issues?

Sorry for closing accidentally 🤦‍♀️

@sareg0 sareg0 closed this Nov 8, 2017
@sareg0 sareg0 reopened this Nov 8, 2017

$navbar-height-reduced: 78px; // some weirdness on smaller screens

nav.navbar {
Copy link
Member Author

Choose a reason for hiding this comment

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

@sareg0 most things happen here. I changed the flex-order of the elements that appear in the header to better fit smaller screens. Some elements were also hidden on mobile devices which are now always visible

@lislis
Copy link
Member Author

lislis commented Nov 8, 2017

@sareg0 sure thing! I left a comment in the files :)

@sareg0
Copy link
Contributor

sareg0 commented Nov 14, 2017

LGTM

@sareg0 sareg0 merged commit e3883d0 into master Nov 14, 2017
@lislis lislis deleted the 519-responsive-styles branch January 9, 2018 19:27
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