From be5aeddd1b3880c3519a40c965b510fd5be9fc2c Mon Sep 17 00:00:00 2001
From: Tyson Gach <tyson@tysongach.com>
Date: Fri, 5 May 2017 17:30:00 -0400
Subject: [PATCH] Tweak primary navigation (#861)

* Tweak primary navigation

- Rename `sidebar` to `navigation`, which is based on its purpose rather
  than its visual placement.
- Use `nav` element.
- Remove unnecessary `ul`; the `nav` provides enough semantics to wrap
  the links in.
- Tweak visual design: show a light background on hover, add more
  whitespace between navigation links and main content.

* Add whitespace around main content box (#862)
---
 .../stylesheets/administrate/application.scss |  2 +-
 .../components/_app-container.scss            |  3 +-
 .../components/_main-content.scss             |  7 ++--
 .../administrate/components/_navigation.scss  | 33 +++++++++++++++++++
 .../administrate/components/_sidebar.scss     | 22 -------------
 .../administrate/library/_variables.scss      |  6 ++--
 .../application/_navigation.html.erb          | 18 ++++++++++
 .../application/_sidebar.html.erb             | 20 -----------
 .../layouts/administrate/application.html.erb |  8 ++---
 docs/customizing_page_views.md                |  2 +-
 .../administrate/views/layout_generator.rb    |  2 +-
 ...r_generator.rb => navigation_generator.rb} |  6 ++--
 .../app/views/layouts/application.html.erb    |  4 +--
 .../{sidebar_spec.rb => navigation_spec.rb}   |  8 ++---
 .../generators/views/layout_generator_spec.rb |  4 +--
 .../views/navigation_generator_spec.rb        | 17 ++++++++++
 .../views/sidebar_generator_spec.rb           | 17 ----------
 17 files changed, 93 insertions(+), 86 deletions(-)
 create mode 100644 app/assets/stylesheets/administrate/components/_navigation.scss
 delete mode 100644 app/assets/stylesheets/administrate/components/_sidebar.scss
 create mode 100644 app/views/administrate/application/_navigation.html.erb
 delete mode 100644 app/views/administrate/application/_sidebar.html.erb
 rename lib/generators/administrate/views/{sidebar_generator.rb => navigation_generator.rb} (56%)
 rename spec/features/{sidebar_spec.rb => navigation_spec.rb} (76%)
 create mode 100644 spec/generators/views/navigation_generator_spec.rb
 delete mode 100644 spec/generators/views/sidebar_generator_spec.rb

diff --git a/app/assets/stylesheets/administrate/application.scss b/app/assets/stylesheets/administrate/application.scss
index d1ab7a2bc0..b80b02f6b8 100644
--- a/app/assets/stylesheets/administrate/application.scss
+++ b/app/assets/stylesheets/administrate/application.scss
@@ -25,7 +25,7 @@
 @import "components/form-actions";
 @import "components/header";
 @import "components/main-content";
+@import "components/navigation";
 @import "components/pagination";
 @import "components/search";
-@import "components/sidebar";
 @import "components/table";
diff --git a/app/assets/stylesheets/administrate/components/_app-container.scss b/app/assets/stylesheets/administrate/components/_app-container.scss
index 5e219af810..b72ca832ad 100644
--- a/app/assets/stylesheets/administrate/components/_app-container.scss
+++ b/app/assets/stylesheets/administrate/components/_app-container.scss
@@ -2,6 +2,7 @@
   @include margin(null auto);
   align-items: stretch;
   display: flex;
-  max-width: 100em;
+  max-width: 100rem;
   min-height: 100vh;
+  padding: $base-spacing;
 }
diff --git a/app/assets/stylesheets/administrate/components/_main-content.scss b/app/assets/stylesheets/administrate/components/_main-content.scss
index 686031939c..4898a0e36d 100644
--- a/app/assets/stylesheets/administrate/components/_main-content.scss
+++ b/app/assets/stylesheets/administrate/components/_main-content.scss
@@ -1,6 +1,7 @@
 .main-content {
   background-color: $white;
-  box-shadow: 0 2px 4px rgba($grey-7, 0.24), 0 2px 6px rgba($grey-7, 0.12);
-  flex: 1;
-  overflow-y: auto;
+  border-radius: $base-border-radius;
+  box-shadow: 0 0 6px 0 rgba($black, 0.12),
+              0 2px 2px rgba($black, 0.2);
+  flex: 1 1 100%;
 }
diff --git a/app/assets/stylesheets/administrate/components/_navigation.scss b/app/assets/stylesheets/administrate/components/_navigation.scss
new file mode 100644
index 0000000000..b15ed266df
--- /dev/null
+++ b/app/assets/stylesheets/administrate/components/_navigation.scss
@@ -0,0 +1,33 @@
+$_navigation-link-padding: 0.6em;
+
+.navigation {
+  flex: 1 0 10rem;
+  padding-bottom: $base-spacing;
+  padding-right: calc(#{$base-spacing} - #{$_navigation-link-padding});
+  padding-top: $base-spacing;
+}
+
+.navigation__link {
+  background-color: transparent;
+  color: $base-font-color;
+  display: block;
+  line-height: 1;
+  margin-left: -($_navigation-link-padding);
+  padding: $_navigation-link-padding;
+  transition: background-color $base-duration $base-timing,
+              color $base-duration $base-timing;
+
+  &:not(:last-of-type) {
+    margin-bottom: $small-spacing;
+  }
+
+  &:hover {
+    background-color: shade($base-background-color, 5%);
+    border-radius: $base-border-radius;
+    color: $base-font-color;
+  }
+}
+
+.navigation__link--active {
+  font-weight: $bold-font-weight;
+}
diff --git a/app/assets/stylesheets/administrate/components/_sidebar.scss b/app/assets/stylesheets/administrate/components/_sidebar.scss
deleted file mode 100644
index 3a042a9cd1..0000000000
--- a/app/assets/stylesheets/administrate/components/_sidebar.scss
+++ /dev/null
@@ -1,22 +0,0 @@
-.sidebar {
-  flex: 0 0 auto;
-  max-width: 12em;
-  overflow-y: auto;
-  padding: 0 $base-spacing $base-spacing;
-}
-
-.sidebar__link {
-  color: $base-font-color;
-  display: block;
-  padding-top: $base-spacing;
-  transition: color 0.05s linear;
-
-  &:hover {
-    color: $blue;
-  }
-}
-
-.sidebar__link--active {
-  color: $blue;
-  font-weight: $bold-font-weight;
-}
diff --git a/app/assets/stylesheets/administrate/library/_variables.scss b/app/assets/stylesheets/administrate/library/_variables.scss
index 5ae7f00d72..ac0ee50a7b 100644
--- a/app/assets/stylesheets/administrate/library/_variables.scss
+++ b/app/assets/stylesheets/administrate/library/_variables.scss
@@ -6,14 +6,14 @@ $heading-font-family: $base-font-family;
 
 $base-font-size: 1em;
 
-$bold-font-weight: 600;
+$bold-font-weight: 700;
 
 $base-line-height: 1.5;
 $heading-line-height: 1.2;
 
 // Other Sizes
 $modular-scale-ratio: $minor-third;
-$base-border-radius: 3px;
+$base-border-radius: 4px;
 $base-spacing: $base-line-height * 1em;
 $small-spacing: $base-spacing / 2;
 
@@ -59,4 +59,4 @@ $base-border: 1px solid $base-border-color;
 
 // Transitions
 $base-duration: 250ms;
-$base-timing: $ease-out-cubic;
+$base-timing: ease-in-out;
diff --git a/app/views/administrate/application/_navigation.html.erb b/app/views/administrate/application/_navigation.html.erb
new file mode 100644
index 0000000000..b0417c87e6
--- /dev/null
+++ b/app/views/administrate/application/_navigation.html.erb
@@ -0,0 +1,18 @@
+<%#
+# Navigation
+
+This partial is used to display the navigation in Administrate.
+By default, the navigation contains navigation links
+for all resources in the admin dashboard,
+as defined by the routes in the `admin/` namespace
+%>
+
+<nav class="navigation">
+  <% Administrate::Namespace.new(namespace).resources.each do |resource| %>
+    <%= link_to(
+      display_resource_name(resource),
+      [namespace, resource.path],
+      class: "navigation__link navigation__link--#{nav_link_state(resource)}"
+    ) %>
+  <% end %>
+</nav>
diff --git a/app/views/administrate/application/_sidebar.html.erb b/app/views/administrate/application/_sidebar.html.erb
deleted file mode 100644
index 30832fe3ac..0000000000
--- a/app/views/administrate/application/_sidebar.html.erb
+++ /dev/null
@@ -1,20 +0,0 @@
-<%#
-# Sidebar
-
-This partial is used to display the sidebar in Administrate.
-By default, the sidebar contains navigation links
-for all resources in the admin dashboard,
-as defined by the routes in the `admin/` namespace
-%>
-
-<ul class="sidebar__list">
-  <% Administrate::Namespace.new(namespace).resources.each do |resource| %>
-    <li>
-      <%= link_to(
-        display_resource_name(resource),
-        [namespace, resource.path],
-        class: "sidebar__link sidebar__link--#{nav_link_state(resource)}"
-      ) %>
-    </li>
-  <% end %>
-</ul>
diff --git a/app/views/layouts/administrate/application.html.erb b/app/views/layouts/administrate/application.html.erb
index b14cd79847..74ff0fa4f4 100644
--- a/app/views/layouts/administrate/application.html.erb
+++ b/app/views/layouts/administrate/application.html.erb
@@ -5,11 +5,11 @@ This view template is used as the layout
 for every page that Administrate generates.
 
 By default, it renders:
-- Sidebar for navigation
+- Navigation
 - Content for a search bar
   (if provided by a `content_for` block in a nested page)
 - Flashes
-- Links to stylesheets and Javascripts
+- Links to stylesheets and JavaScripts
 %>
 
 <!DOCTYPE html>
@@ -27,9 +27,7 @@ By default, it renders:
 
   <div class="app-container">
 
-    <div class="sidebar">
-      <%= render "sidebar" -%>
-    </div>
+    <%= render "navigation" -%>
 
     <main class="main-content" role="main">
       <%= content_for(:search) %>
diff --git a/docs/customizing_page_views.md b/docs/customizing_page_views.md
index 8e24ca7e7d..4638254ab0 100644
--- a/docs/customizing_page_views.md
+++ b/docs/customizing_page_views.md
@@ -72,7 +72,7 @@ It's so easy that pass in the "layout" key word to the view generators.
 ```bash
 rails generate administrate:views:layout
  # -> app/views/layouts/admin/application.html.erb
- # -> app/views/admin/application/_sidebar.html.erb
+ # -> app/views/admin/application/_navigation.html.erb
  # -> app/views/admin/application/_javascript.html.erb
  # -> app/views/admin/application/_flashes.html.erb
 ```
diff --git a/lib/generators/administrate/views/layout_generator.rb b/lib/generators/administrate/views/layout_generator.rb
index 8b2feeefc3..afe8b0cacc 100644
--- a/lib/generators/administrate/views/layout_generator.rb
+++ b/lib/generators/administrate/views/layout_generator.rb
@@ -12,7 +12,7 @@ def copy_template
             "app/views/layouts/admin/application.html.erb",
           )
 
-          call_generator("administrate:views:sidebar")
+          call_generator("administrate:views:navigation")
           copy_resource_template("_javascript")
           copy_resource_template("_flashes")
         end
diff --git a/lib/generators/administrate/views/sidebar_generator.rb b/lib/generators/administrate/views/navigation_generator.rb
similarity index 56%
rename from lib/generators/administrate/views/sidebar_generator.rb
rename to lib/generators/administrate/views/navigation_generator.rb
index 91cbc8ab96..e35570cf4c 100644
--- a/lib/generators/administrate/views/sidebar_generator.rb
+++ b/lib/generators/administrate/views/navigation_generator.rb
@@ -3,11 +3,11 @@
 module Administrate
   module Generators
     module Views
-      class SidebarGenerator < Administrate::ViewGenerator
+      class NavigationGenerator < Administrate::ViewGenerator
         source_root template_source_path
 
-        def copy_sidebar
-          copy_resource_template("_sidebar")
+        def copy_navigation
+          copy_resource_template("_navigation")
         end
       end
     end
diff --git a/spec/example_app/app/views/layouts/application.html.erb b/spec/example_app/app/views/layouts/application.html.erb
index f59e26da61..eee19ec0d9 100644
--- a/spec/example_app/app/views/layouts/application.html.erb
+++ b/spec/example_app/app/views/layouts/application.html.erb
@@ -17,9 +17,7 @@
 
 <body class="<%= body_class %>">
   <main class="main">
-    <div class="sidebar">
-      <%= render "sidebar" -%>
-    </div>
+    <%= render "navigation" -%>
 
     <div class="content">
       <%= render "flashes" -%>
diff --git a/spec/features/sidebar_spec.rb b/spec/features/navigation_spec.rb
similarity index 76%
rename from spec/features/sidebar_spec.rb
rename to spec/features/navigation_spec.rb
index a38903f5ac..acac84f6e6 100644
--- a/spec/features/sidebar_spec.rb
+++ b/spec/features/navigation_spec.rb
@@ -1,10 +1,10 @@
 require "rails_helper"
 
-describe "sidebar" do
+describe "navigation" do
   it "highlights the link to the current page's resource type" do
     visit admin_customers_path
 
-    active_link = find(".sidebar__link--active")
+    active_link = find(".navigation__link--active")
 
     expect(active_link.text).to eq "Customers"
   end
@@ -24,8 +24,8 @@
     with_translations(:en, translations) do
       visit admin_customers_path
 
-      sidebar = find(".sidebar__list")
-      expect(sidebar).to have_link("Users")
+      navigation = find(".navigation")
+      expect(navigation).to have_link("Users")
       expect(page).to have_header("Users")
     end
   end
diff --git a/spec/generators/views/layout_generator_spec.rb b/spec/generators/views/layout_generator_spec.rb
index 885de12a49..3e0ea4893e 100644
--- a/spec/generators/views/layout_generator_spec.rb
+++ b/spec/generators/views/layout_generator_spec.rb
@@ -27,13 +27,13 @@
       expect(contents).to eq(expected_contents)
     end
 
-    it "copies the sidebar partial into the `admin/application` namespace" do
+    it "copies the navigation partial into the `admin/application` namespace" do
       allow(Rails::Generators).to receive(:invoke)
 
       run_generator []
 
       expect(Rails::Generators).
-        to invoke_generator("administrate:views:sidebar")
+        to invoke_generator("administrate:views:navigation")
     end
 
     it "copies the javascript partial into the `admin/application` namespace" do
diff --git a/spec/generators/views/navigation_generator_spec.rb b/spec/generators/views/navigation_generator_spec.rb
new file mode 100644
index 0000000000..d75cf5c3d0
--- /dev/null
+++ b/spec/generators/views/navigation_generator_spec.rb
@@ -0,0 +1,17 @@
+require "spec_helper"
+require "generators/administrate/views/navigation_generator"
+require "support/generator_spec_helpers"
+
+describe Administrate::Generators::Views::NavigationGenerator, :generator do
+  describe "administrate:views:navigation" do
+    it "copies the navigation partial into the `admin/application` namespace" do
+      expected_contents = contents_for_application_template("_navigation")
+      generated_file = file("app/views/admin/application/_navigation.html.erb")
+
+      run_generator []
+
+      contents = File.read(generated_file)
+      expect(contents).to eq(expected_contents)
+    end
+  end
+end
diff --git a/spec/generators/views/sidebar_generator_spec.rb b/spec/generators/views/sidebar_generator_spec.rb
deleted file mode 100644
index 7af7a1bc77..0000000000
--- a/spec/generators/views/sidebar_generator_spec.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-require "spec_helper"
-require "generators/administrate/views/sidebar_generator"
-require "support/generator_spec_helpers"
-
-describe Administrate::Generators::Views::SidebarGenerator, :generator do
-  describe "administrate:views:sidebar" do
-    it "copies the sidebar partial into the `admin/application` namespace" do
-      expected_contents = contents_for_application_template("_sidebar")
-      generated_file = file("app/views/admin/application/_sidebar.html.erb")
-
-      run_generator []
-
-      contents = File.read(generated_file)
-      expect(contents).to eq(expected_contents)
-    end
-  end
-end