From 150d791c056d870c80feb5b7f609be45499ae5d0 Mon Sep 17 00:00:00 2001 From: Tyson Gach Date: Wed, 31 May 2017 11:35:58 -0400 Subject: [PATCH] Redesign app navigation - Remove sticky header; it caused shifting in the nav's height which can be distracting. - Wrap the navigation in a `nav` element (rather than `header`) which is more semantically correct and maps to the `navigation` role in the accessibility tree. - Component-ize CSS: `avatar`, `logo` and `icon` are now reusable components. - Begin using SVG `symbol` for icon and SVG assets, instead of Font Awesome or using the SVG through an `img` tag. This allows for CSS styles (e.g. `fill` color) and _greatly_ improves accessibility. - Remove Twitter link (it is still listed in the footer). - Maintain Pricing and Sign In with GitHub links across all breakpoints instead of removing them on mobile. - Vastly simplify CSS, including removal of over-qualified selectors (e.g. `ul li a.auth`). - Remove the need for page-based classes via `content_for :header_class`. --- app/assets/images/logo.svg | 1 - app/assets/javascripts/application.js | 2 - app/assets/javascripts/sticky-header.js | 5 - app/assets/stylesheets/application.scss | 1 + app/assets/stylesheets/base/_helpers.scss | 4 - app/assets/stylesheets/base/_variables.scss | 5 +- .../stylesheets/components/_allowance.scss | 1 + .../stylesheets/components/_app-nav.scss | 60 ++++++ .../stylesheets/components/_avatar.scss | 24 +++ .../stylesheets/components/_button.scss | 3 +- .../stylesheets/components/_components.scss | 5 +- .../stylesheets/components/_header.scss | 189 ------------------ app/assets/stylesheets/components/_icon.scss | 4 + app/assets/stylesheets/components/_logo.scss | 14 ++ .../stylesheets/utilities/_line-height.scss | 3 + app/assets/stylesheets/utilities/_margin.scss | 3 + .../stylesheets/utilities/_utilities.scss | 2 + app/views/application/_app_nav.haml | 50 +++++ app/views/application/_header.haml | 50 ----- app/views/home/index.haml | 4 +- app/views/layouts/application.haml | 2 +- app/views/repos/index.haml | 1 - public/images/icons.svg | 11 + spec/features/admin_masquerades_spec.rb | 4 +- 24 files changed, 185 insertions(+), 263 deletions(-) delete mode 100644 app/assets/images/logo.svg delete mode 100644 app/assets/javascripts/sticky-header.js create mode 100644 app/assets/stylesheets/components/_app-nav.scss create mode 100644 app/assets/stylesheets/components/_avatar.scss delete mode 100644 app/assets/stylesheets/components/_header.scss create mode 100644 app/assets/stylesheets/components/_icon.scss create mode 100644 app/assets/stylesheets/components/_logo.scss create mode 100644 app/assets/stylesheets/utilities/_line-height.scss create mode 100644 app/assets/stylesheets/utilities/_margin.scss create mode 100644 app/assets/stylesheets/utilities/_utilities.scss create mode 100644 app/views/application/_app_nav.haml delete mode 100644 app/views/application/_header.haml create mode 100644 public/images/icons.svg diff --git a/app/assets/images/logo.svg b/app/assets/images/logo.svg deleted file mode 100644 index 4554b7d80..000000000 --- a/app/assets/images/logo.svg +++ /dev/null @@ -1 +0,0 @@ -Hound logo diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index f95f6f6da..8bf593797 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -7,5 +7,3 @@ //= require react_ujs //= require classnames - -//= require sticky-header diff --git a/app/assets/javascripts/sticky-header.js b/app/assets/javascripts/sticky-header.js deleted file mode 100644 index 89d4a9757..000000000 --- a/app/assets/javascripts/sticky-header.js +++ /dev/null @@ -1,5 +0,0 @@ -$(document).ready(function() { - $(window).on("scroll touchmove", function () { - $('.global-header.repo-index').toggleClass('shrunk', $(document).scrollTop() > 0); - }); -}); diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 3ca5e0783..5f7f30d33 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -6,3 +6,4 @@ @import "base/base"; @import "components/components"; @import "pages/pages"; +@import "utilities/utilities"; diff --git a/app/assets/stylesheets/base/_helpers.scss b/app/assets/stylesheets/base/_helpers.scss index ca310553f..a3fb6f4df 100644 --- a/app/assets/stylesheets/base/_helpers.scss +++ b/app/assets/stylesheets/base/_helpers.scss @@ -19,10 +19,6 @@ .content { @include outer-container; - - &.repo-index { - padding-top: 10em; - } } .inner-wrapper { diff --git a/app/assets/stylesheets/base/_variables.scss b/app/assets/stylesheets/base/_variables.scss index 25cfd3957..0032553b7 100644 --- a/app/assets/stylesheets/base/_variables.scss +++ b/app/assets/stylesheets/base/_variables.scss @@ -18,6 +18,7 @@ $light-gold: lighten($gold, 50%); $tan: rgb(255, 220, 173); $white: #fff; $black: #000; +$light-gray: rgb(230, 230, 230); $ruby: rgb(193, 30, 29); $javascript: rgb(246, 221, 0); @@ -54,7 +55,6 @@ $medium-large-screen-size: 57.5em; $large-screen-size: 920px; $small-screen-only: new-breakpoint(max-width $small-screen-size 2); -$medium-screen-down--header: new-breakpoint(max-width ($medium-header-breakpoint - 1px) 4); $medium-screen-down: new-breakpoint(max-width ($medium-screen-size - 1px) 4); $medium-screen-up: new-breakpoint(min-width $medium-screen-size 12); $medium-large-screen-only: new-breakpoint(max-width $medium-large-screen-size 8); @@ -86,6 +86,3 @@ $font-size-xxlarge: 3.8em; $base-transition-timing: 0.2s; $base-easing: cubic-bezier(0.39, 0.575, 0.565, 1); $base-transition: all $base-transition-timing $base-easing; - -// Z-Indices -$z-index-front: 100; diff --git a/app/assets/stylesheets/components/_allowance.scss b/app/assets/stylesheets/components/_allowance.scss index 61ca814e8..d7c8ed9de 100644 --- a/app/assets/stylesheets/components/_allowance.scss +++ b/app/assets/stylesheets/components/_allowance.scss @@ -3,6 +3,7 @@ border-radius: 100px; color: $base-accent-color; line-height: 40px; + margin-right: $base-spacing; padding: 0 $small-spacing; strong { diff --git a/app/assets/stylesheets/components/_app-nav.scss b/app/assets/stylesheets/components/_app-nav.scss new file mode 100644 index 000000000..59f161ac2 --- /dev/null +++ b/app/assets/stylesheets/components/_app-nav.scss @@ -0,0 +1,60 @@ +$_app-nav-layout-breakpoint: 42em; + +.app-nav { + font-weight: $font-weight-normal; + + a { + color: $base-font-color; + + &:hover { + color: $base-accent-color; + } + } +} + +.app-nav__container { + align-items: center; + display: flex; + flex-direction: column; + padding: 2rem; + + @media (min-width: $_app-nav-layout-breakpoint) { + flex-direction: row; + } +} + +.app-nav__logo { + display: block; + + @media (max-width: $_app-nav-layout-breakpoint) { + margin-bottom: $base-spacing; + } + + @media (min-width: $_app-nav-layout-breakpoint) { + margin-right: $base-spacing; + } +} + +.app-nav__item:not(:first-of-type) { + margin-left: $base-spacing; +} + +.app-nav__app-items { + @media (max-width: $_app-nav-layout-breakpoint) { + margin-bottom: $base-spacing; + } +} + +.app-nav__user-items { + align-items: center; + display: flex; + + @media (min-width: $_app-nav-layout-breakpoint) { + margin-left: auto; + } +} + +.app-nav__account-link { + align-items: center; + display: flex; +} diff --git a/app/assets/stylesheets/components/_avatar.scss b/app/assets/stylesheets/components/_avatar.scss new file mode 100644 index 000000000..8209b876e --- /dev/null +++ b/app/assets/stylesheets/components/_avatar.scss @@ -0,0 +1,24 @@ +$_avatar-size: 2rem; + +.avatar { + @include size($_avatar-size); + background-color: $light-gray; + border-radius: 50%; + display: inline-block; + overflow: hidden; + position: relative; + vertical-align: middle; + + &::after { + @include position(absolute, 0); + border-radius: 50%; + box-shadow: inset 0 0 0 1px rgba($black, 0.1); + content: ""; + pointer-events: none; + } + + img { + @include size($_avatar-size); + object-fit: cover; + } +} diff --git a/app/assets/stylesheets/components/_button.scss b/app/assets/stylesheets/components/_button.scss index da2f31848..3e93d04f1 100644 --- a/app/assets/stylesheets/components/_button.scss +++ b/app/assets/stylesheets/components/_button.scss @@ -8,7 +8,6 @@ font-family: $font-family-default; font-size: modular-scale(0); font-weight: $font-weight-normal; - line-height: 1; padding: modular-scale(-2) modular-scale(1); text-decoration: none; transition: background-color $base-transition-timing $base-easing, @@ -56,4 +55,6 @@ .button__icon { display: inline-block; margin-right: modular-scale(-6); + position: relative; + top: modular-scale(-11); } diff --git a/app/assets/stylesheets/components/_components.scss b/app/assets/stylesheets/components/_components.scss index 6362198c6..9126ae36d 100644 --- a/app/assets/stylesheets/components/_components.scss +++ b/app/assets/stylesheets/components/_components.scss @@ -1,9 +1,12 @@ @import "allowance"; +@import "app-nav"; +@import "avatar"; @import "button"; @import "code"; @import "footer"; -@import "header"; +@import "icon"; @import "inline-flash"; +@import "logo"; @import "plan-markers"; @import "pricing"; @import "toggle_switch"; diff --git a/app/assets/stylesheets/components/_header.scss b/app/assets/stylesheets/components/_header.scss deleted file mode 100644 index 0173b21ca..000000000 --- a/app/assets/stylesheets/components/_header.scss +++ /dev/null @@ -1,189 +0,0 @@ -.global-header { - background-color: $white; - height: 120px; - transition: height 0.2s ease-in-out; - width: 100%; - - @include media($medium-screen-down--header) { - height: auto; - padding: 1em 2em; - text-align: center; - } - - &.repo-index { - position: fixed; - z-index: $z-index-front; - } - - &.shrunk { - border-bottom: 2px solid $base-accent-color; - height: 80px; - - @include media($medium-screen-down--header) { - height: auto; - padding: 0; - } - - @include media($small-screen-only) { - padding: 0.5em; - } - } -} - -.global-header-wrapper { - @include outer-container; - height: 100%; - padding: 0 0.5em; - - @include media($medium-screen-down--header) { - height: auto; - padding: 2em 1em; - text-align: center; - } - - &.home-header { - @include clearfix; - margin: 0; - max-width: none; - padding: 2em; - width: 100%; - - h1 { - background-image: image-url("logo.svg"); - height: 40px; - width: 174px; - } - } - - .logo { - display: table; - height: 100%; - margin-right: 1em; - - @include media($medium-screen-down--header) { - display: inline-block; - float: none; - vertical-align: middle; - } - - @include media($small-screen-only) { - display: table; - margin: 0 0 1em; - width: 100%; - } - - .va-cell { - font-size: 0; - } - } - - .logo-link { - display: inline-block; - - &:hover { - border: none; - } - } - - h1 { - @include hide-text; - background-image: image-url("icon.svg"); - background-position: center center; - background-repeat: no-repeat; - background-size: 100%; - height: 50px; - margin: 0; - width: 51px; - } - - ul { - display: table; - height: 100%; - - @include media($medium-screen-down--header) { - display: inline-block; - - &.pull-right, - &.pull-left { - float: none; - } - } - - + ul { - @include media($medium-screen-down--header) { - padding: 1.2em 1em; - } - - @include media($small-screen-only) { - display: none; - } - } - - li { - display: table-cell; - text-align: center; - vertical-align: middle; - - @include media($medium-screen-down--header) { - display: inline-block; - } - - + li { - padding-left: 1em; - } - - a { - color: $base-font-color; - display: inline-block; - font-weight: $font-weight-light; - letter-spacing: 1px; - - &:hover { - color: $base-accent-color; - } - - &.auth, - &.docs, - &.faq { - border: 1px solid $base-border-color; - border-radius: 26px; - line-height: 40px; - padding: 0em 1.4em; - - &:hover { - border-color: $base-accent-color; - } - - i.fa { - display: inline-block; - margin-right: 0.2em; - vertical-align: middle; - } - - span { - display: inline-block; - padding-bottom: 2px; - vertical-align: middle; - } - } - - &.account { - .avatar { - @include size(38px); - background: transparent no-repeat 50% 50%; - background-size: cover; - border-radius: 50%; - display: inline-block; - vertical-align: middle; - } - - span { - display: inline-block; - margin-left: 0.2em; - vertical-align: middle; - } - } - } - } - } -} diff --git a/app/assets/stylesheets/components/_icon.scss b/app/assets/stylesheets/components/_icon.scss new file mode 100644 index 000000000..bb802044f --- /dev/null +++ b/app/assets/stylesheets/components/_icon.scss @@ -0,0 +1,4 @@ +.icon { + @include size(1em); + fill: currentcolor; +} diff --git a/app/assets/stylesheets/components/_logo.scss b/app/assets/stylesheets/components/_logo.scss new file mode 100644 index 000000000..4e9f1843f --- /dev/null +++ b/app/assets/stylesheets/components/_logo.scss @@ -0,0 +1,14 @@ +$_logo-aspect-ratio: 217 / 50; +$_logo-height: 2.5rem; +$_logo-color: $purple; + +.logo { + fill: $_logo-color; + height: $_logo-height; + transition: fill $base-transition-timing $base-easing; + width: $_logo-height * $_logo-aspect-ratio; + + &:hover { + fill: shade($_logo-color, 15%); + } +} diff --git a/app/assets/stylesheets/utilities/_line-height.scss b/app/assets/stylesheets/utilities/_line-height.scss new file mode 100644 index 000000000..06226ec66 --- /dev/null +++ b/app/assets/stylesheets/utilities/_line-height.scss @@ -0,0 +1,3 @@ +.line-height-zero { + line-height: 0; +} diff --git a/app/assets/stylesheets/utilities/_margin.scss b/app/assets/stylesheets/utilities/_margin.scss new file mode 100644 index 000000000..b8de025eb --- /dev/null +++ b/app/assets/stylesheets/utilities/_margin.scss @@ -0,0 +1,3 @@ +.margin-right-x-small { + margin-right: $xsmall-spacing; +} diff --git a/app/assets/stylesheets/utilities/_utilities.scss b/app/assets/stylesheets/utilities/_utilities.scss new file mode 100644 index 000000000..cae0c91bd --- /dev/null +++ b/app/assets/stylesheets/utilities/_utilities.scss @@ -0,0 +1,2 @@ +@import "line-height"; +@import "margin"; diff --git a/app/views/application/_app_nav.haml b/app/views/application/_app_nav.haml new file mode 100644 index 000000000..c8203e4b6 --- /dev/null +++ b/app/views/application/_app_nav.haml @@ -0,0 +1,50 @@ +%nav.app-nav{ role: "navigation" } + .app-nav__container + = link_to repos_path, class: "line-height-zero" do + %svg.logo.app-nav__logo{ role: "img" } + %title Hound + %use{ "xlink:href" => "/images/icons.svg#hound" } + + .app-nav__app-items + = link_to "Docs", configuration_path, class: "app-nav__item" + + = link_to "FAQ", + "https://intercom.help/hound/frequently-asked-questions", + class: "app-nav__item" + + - unless signed_in? + = link_to "Pricing", + root_path(anchor: "pricing"), + class: "app-nav__item" + + .app-nav__user-items + - if signed_in? + - if current_user.subscriptions.count > 0 + .allowance{ "data-role" => "allowance-container" } + Private Repos + %strong + %span{ "data-role" => "subscribed-repo-count" } + #{current_user.subscribed_repos.count} + \/ + %span{ "data-role" => "tier-allowance" } + #{current_user.plan_max} + + = link_to account_path, class: "app-nav__item app-nav__account-link" do + - if current_user.email.present? + .avatar.margin-right-x-small + = image_tag avatar_url(current_user), alt: "" + = current_user.username + + - if masquerading? + = link_to t("stop_masquerading"), + admin_masquerade_path(current_user.username), + method: :delete, + class: "app-nav__item" + - else + = link_to t("sign_out"), sign_out_path, class: "app-nav__item" + + - else + = link_to "/auth/github", class: "button " do + %svg.icon.button__icon{ "aria-hidden" => "true" } + %use{ "xlink:href" => "/images/icons.svg#github" } + = t("authenticate") diff --git a/app/views/application/_header.haml b/app/views/application/_header.haml deleted file mode 100644 index 633cb39a7..000000000 --- a/app/views/application/_header.haml +++ /dev/null @@ -1,50 +0,0 @@ -.global-header{ class: content_for(:header_class) } - %header.global-header-wrapper{ class: content_for(:header_class) } - .logo.pull-left - .va-cell - = link_to repos_path, class: "logo-link" do - %h1 - Hound - %ul.pull-left - %li - = link_to "Docs", configuration_path, class: "docs" - %li - = link_to "FAQ", - "https://intercom.help/hound/frequently-asked-questions", - class: "faq" - %ul.pull-right{ "data-role" => "account-actions" } - - if signed_in? - - if current_user.subscriptions.count > 0 - %li - .allowance{ "data-role" => "allowance-container" } - Private Repos - %strong - %span{ "data-role" => "subscribed-repo-count" } - #{current_user.subscribed_repos.count} - \/ - %span{ "data-role" => "tier-allowance" } - #{current_user.plan_max} - %li - = link_to account_path, class: "account" do - - if current_user.email.present? - - avatar = avatar_url(current_user) - .avatar{ style: "background-image: url('#{avatar}')" } - %span= current_user.username - %li - - if masquerading? - = link_to t("stop_masquerading"), - admin_masquerade_path(current_user.username), - method: :delete - - else - = link_to t("sign_out"), sign_out_path, class: "sign-out" - - else - %li - = link_to "Pricing", root_path(anchor: "pricing"), class: "pricing" - %li - - options = new_window_options(class: "tweet-us") - = link_to "https://twitter.com/houndci", options do - %span @houndci - %li.auth-container - = link_to "/auth/github", class: "button" do - %i.fa.fa-github.button__icon - = t("authenticate") diff --git a/app/views/home/index.haml b/app/views/home/index.haml index eeae4535a..858d898d2 100644 --- a/app/views/home/index.haml +++ b/app/views/home/index.haml @@ -1,7 +1,6 @@ - content_for :page_title, I18n.t("home.index.title") - content_for :meta_description, I18n.t("home.index.description") - content_for :body_class, "home home-index" -- content_for :header_class, "home-header" %section .section-content @@ -16,7 +15,8 @@ .home-hero-cta = link_to "/auth/github", class: "button button--primary button--large" do - %i.fa.fa-github.button__icon + %svg.icon.button__icon{ "aria-hidden" => "true" } + %use{ "xlink:href" => "/images/icons.svg#github" } = t("authenticate") .home-languages diff --git a/app/views/layouts/application.haml b/app/views/layouts/application.haml index 4ea118a51..eeb6def75 100644 --- a/app/views/layouts/application.haml +++ b/app/views/layouts/application.haml @@ -14,7 +14,7 @@ = javascript_include_tag "application" = csrf_meta_tags %body{ class: content_for(:body_class) || "hound" } - = render "application/header" + = render "application/app_nav" - user_facing_flashes.each do |key, value| .flash{ class: key } = value diff --git a/app/views/repos/index.haml b/app/views/repos/index.haml index 926579c40..7cf3e3f1a 100644 --- a/app/views/repos/index.haml +++ b/app/views/repos/index.haml @@ -1,5 +1,4 @@ - content_for :page_title, 'Your Repos' -- content_for :header_class, "repo-index" - content_for :container_class, "repo-index" = render 'settings' diff --git a/public/images/icons.svg b/public/images/icons.svg new file mode 100644 index 000000000..f675366e8 --- /dev/null +++ b/public/images/icons.svg @@ -0,0 +1,11 @@ + + + GitHub + + + + + Hound + + + diff --git a/spec/features/admin_masquerades_spec.rb b/spec/features/admin_masquerades_spec.rb index 0b9b991a4..520107d7a 100644 --- a/spec/features/admin_masquerades_spec.rb +++ b/spec/features/admin_masquerades_spec.rb @@ -12,13 +12,13 @@ visit admin_masquerade_path(user.username) expect(current_path).to eq(repos_path) - within "header .account" do + within ".app-nav" do expect(page).to have_text(user.username) end click_on "Stop Masquerading" - within "header .account" do + within ".app-nav" do expect(page).to have_text(admin.username) end end