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.

Sign up for GitHub

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

Make the sidebar menu collapsible #5

Closed
wants to merge 13 commits into from
+283 −57
Closed

Make the sidebar menu collapsible #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
27bdef1
Add new mixin and variables for media queries
davidedistefano Jan 30, 2017
55a168d
Improve navigation on smaller screens
davidedistefano Jan 31, 2017
51c5893
Update admin layout media queries
davidedistefano Jan 31, 2017
2d112fa
Add mobile hamburger menu
davidedistefano Feb 1, 2017
0623804
Truncate breadcrumb text on small screen sizes
davidedistefano Feb 2, 2017
321cdac
Add new div containers to wrap admin content
davidedistefano Feb 2, 2017
b9d3fd0
Update admin flash messages layout
davidedistefano Feb 6, 2017
8ecde31
Restore skeleton layout
davidedistefano Feb 6, 2017
14f07bc
Add min-height to admin wrapper to prevent bounce effect
davidedistefano Feb 6, 2017
ed5f03f
Fix admin scroll behavior
davidedistefano Feb 6, 2017
69eb16a
Refactor collapsible menu code
davidedistefano Feb 7, 2017
9e6ba1a
Remove Bourbon deprecated mixins
davidedistefano Feb 7, 2017
f43ceb7
Update header logo height
davidedistefano Feb 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions backend/app/assets/javascripts/spree/backend/navigation.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,18 @@ navHeight = ->
checkSideBarFit = ->
$('.admin-nav').toggleClass('fits', navHeight() < $(window).height())

$ ->
$(".admin-nav-sticky, .admin-nav").stick_in_parent()
checkSticky = ->
stickyElements = $('.admin-nav-sticky, .admin-nav')

if $(window).width() < 768
stickyElements.trigger 'sticky_kit:detach'
else
stickyElements.stick_in_parent(spacer: false)

adjustNavigation = ->
checkSideBarFit()
$(window).on('resize', checkSideBarFit)
checkSticky()

$ ->
adjustNavigation()
$(window).on('resize', adjustNavigation)
13 changes: 13 additions & 0 deletions backend/app/assets/stylesheets/spree/backend/components/_breadcrumb.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,17 @@
.breadcrumb {
font-size: 16px;
margin-bottom: 0;
@include media($tablet) {
font-size: 14px;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}

li {
@include media($tablet) {
float: none;
display: inline;
}
}
}
6 changes: 6 additions & 0 deletions backend/app/assets/stylesheets/spree/backend/components/_messages.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@
.flash-wrapper {
@include position(fixed, null 0 0 $width-sidebar);
z-index: 1000;
@include media($desktop) {
left: $width-sidebar-collapsed;
}
@include media($tablet) {
left: 0;
}
}

.flash {
Expand Down
154 changes: 129 additions & 25 deletions backend/app/assets/stylesheets/spree/backend/components/_navigation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,49 @@ $padding-x-navbar: 26px;
$padding-y-navbar: 13px;
$padding-y-navbar-submenu: 9px;

.menu-button-icon {
width: 34px;
height: 32px;
margin-right: 10px;
background: $color-btn-bg;
color: $color-btn-text;
border-radius: 4px;
text-align: center;
cursor: pointer;
display: none;
&:hover {
background: $color-btn-hover-bg;
}
@include media($tablet) {
display: block;
}

i {
padding-top: 7px;
color: $color-btn-text;
font-size: 18px;
}
}

#menu-button {
display: none;

&:checked ~ .admin-wrapper {
overflow: hidden;

.admin-nav {
transform: translateX(0);
}

.admin-content {
transform: translateX($width-sidebar);
@include media($large-device) {
transform: none;
}
}
}
}

nav.menu {
ul {
list-style: none;
Expand Down Expand Up @@ -30,18 +73,39 @@ nav.menu {
border-right: $border-sidebar;
background: $color-sidebar-bg;
z-index: $z-index-navbar-flyout;
@include media($desktop) {
width: $width-sidebar-collapsed;
}
@include media($tablet) {
width: $width-sidebar;
position: fixed;
overflow-y: auto;
overflow-x: hidden;
-webkit-overflow-scrolling: touch;
transform: translateX(-$width-sidebar)
}
}

.admin-nav-header {
padding: 15px;
height: 58px; // height of .page-title
height: 47px; // height of .page-title
box-sizing: content-box;
background-color: $color-1;
border-bottom: 1px solid $color-border;
@include media($tablet) {
height: 44px;
}

a {
display: inline-block;
line-height: 58px;
line-height: 47px;
@include media($desktop) {
display: none;
}
@include media($tablet) {
display: inline-block;
Copy link

Choose a reason for hiding this comment

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

If it's inline-block by default do we need this additional media query?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since it is not mobile first approach, we need to reset the display property.

line-height: 44px;
}
}

img {
Expand Down Expand Up @@ -69,40 +133,54 @@ nav.menu {

&.selected {
background: $color-navbar-active-bg;
@include media($desktop) {
position: relative;
}

> a {
color: $color-navbar-active;
&:not(:hover) > ul {
@include media($desktop) {
display: none;
}
@include media($tablet) {
display: block;
}
}
}

&.selected li {
padding-left: 2em;
}
> ul {
@include media($desktop) {
@include flyout;
}
@include media($tablet) {
position: relative;
left: 0;
width: $width-sidebar;
border-top: 0;
border-bottom: 0;
}
}

&:not(.selected):not(:hover) > ul {
display: none;
li {
padding-left: 2em;
}

> a {
color: $color-navbar-active;
}
}

&:not(.selected) {
position: relative;

// flyout nav
> ul {
position: absolute;
top: 0;
left: 100%;
width: $width-sidebar-flyout;
margin-left: 0;
border: $border-sidebar;
background: $color-navbar-submenu-bg;
box-shadow: $box-shadow;
@include caret($direction: left, $color-caret: $color-navbar-submenu-bg);

&:before {
z-index: 1;
top: 1.5em;
@include flyout;
@include media($tablet) {
display: none;
}
}

&:not(:hover) > ul {
display: none;
}
}
}
}

Expand All @@ -115,6 +193,12 @@ nav.menu {

.icon_link {
text-indent: 2em;
@include media($desktop) {
text-indent: 1.5em;
}
@include media($tablet) {
text-indent: 2em;
}

&:before {
position: absolute;
Expand All @@ -128,6 +212,13 @@ nav.menu {
font-family: $base-font-family;
-webkit-font-smoothing: auto;
-moz-osx-font-smoothing: auto;
@include media($desktop) {
overflow: hidden;
display: block;
}
@include media($tablet) {
overflow: visible;
}
}

.admin-subnav {
Expand Down Expand Up @@ -173,6 +264,13 @@ nav.menu {
width: $width-sidebar;
border-top: $border-sidebar;
border-right: $border-sidebar;
@include media($desktop) {
width: $width-sidebar-collapsed;
}
@include media($tablet) {
width: $width-sidebar;
position: relative;
}
}

.admin-login-nav {
Expand All @@ -182,6 +280,12 @@ nav.menu {

li {
padding: 0.3rem 1.2rem;
@include media($desktop) {
padding: 0.3rem 0.5rem 0.3rem 1.1rem;
}
@include media($tablet) {
padding: 0.3rem 1.2rem;
}
}

a {
Expand Down
2 changes: 2 additions & 0 deletions backend/app/assets/stylesheets/spree/backend/globals/_mixins.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
@import "mixins/caret";
@import "mixins/flyout";
@import "mixins/line_through";
@import "mixins/media";
12 changes: 12 additions & 0 deletions backend/app/assets/stylesheets/spree/backend/globals/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ $font-weight-normal: 400 !default;

$width-sidebar: 200px !default;
$width-sidebar-flyout: 225px !default;
$width-sidebar-collapsed: 42px !default;

// Stacking
//--------------------------------------------------------------
Expand All @@ -206,3 +207,14 @@ $z-index-navbar-flyout: 1000 !default;
// Sidebar
//--------------------------------------------------------------
$border-sidebar: 1px solid $color-sidebar-border !default;

// Media queries
//--------------------------------------------------------------
$desktop-width: 959px;
$tablet-width: 768px;
$mobile-width: 480px;

$large-device: "only screen and (min-width: #{$tablet-width})";
$desktop: "only screen and (max-width: #{$desktop-width})";
$tablet: "only screen and (max-width: #{$tablet-width})";
$mobile: "only screen and (max-width: #{$mobile-width})";
Copy link

Choose a reason for hiding this comment

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

Prefer always going up for media queries (making sure to always use min-width). This helps create a mobile first approach:

$tablet: "screen and (min-width: 481px)";
$desktop: "screen and (min-width: 769px)";

.thing {
  // default styles + mobile
  style: style;

  @media $tablet {
    // more things
  }

  @media $desktop {
    // more things
  }
}

Copy link
Member

Choose a reason for hiding this comment

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

@graygilmore for now we just converted the skeleton media queries set here. We internally discussed about changing them to a mobile first approach but it seemed like something not related to this PR and probably something that need a better testing.

Copy link

Choose a reason for hiding this comment

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

👍 I also only just realized this wasn't a PR into Solidus itself. 🤦‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

it will be soon so a review at this stage is very helpful, thanks @graygilmore

16 changes: 16 additions & 0 deletions backend/app/assets/stylesheets/spree/backend/globals/mixins/_flyout.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@mixin flyout {
Copy link
Member

Choose a reason for hiding this comment

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

as said above, I'd remove this mixin in favor of a better code organization.

position: absolute;
top: 0;
left: 100%;
width: $width-sidebar-flyout;
margin-left: 0;
border: $border-sidebar;
background: $color-navbar-submenu-bg;
box-shadow: $box-shadow;
@include caret($direction: left, $color-caret: $color-navbar-submenu-bg);

&:before {
z-index: 1;
top: 1.5em;
}
}
28 changes: 28 additions & 0 deletions backend/app/assets/stylesheets/spree/backend/globals/mixins/_media.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/// Outputs a media-query block.
///
/// @param {String} $media-query
/// A string corresponding to the media query to target.
///
/// @example scss - Usage
/// .responsive-element {
/// color: blue;
/// @include media("screen and (min-width: 480px)") {
/// color: red;
/// }
/// }
///
/// @example css - CSS Output
/// .responsive-element {
/// color: blue;
/// }
/// @media screen and (min-width: 480px) {
/// .responsive-element {
/// color: red;
/// }
/// }

@mixin media($media-query) {
@media #{$media-query} {
@content;
}
}
7 changes: 7 additions & 0 deletions backend/app/assets/stylesheets/spree/backend/shared/_header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@

body:not(.new-layout) & {
margin-left: $width-sidebar;
@include media($desktop) {
margin-left: $width-sidebar-collapsed;
}
@include media($tablet) {
margin-left: 0;
}
}

@media print { display: none }
Expand All @@ -26,6 +32,7 @@

.header-actions {
flex-grow: 1;
padding: 0;
text-align: right;
line-height: 38px;

Expand Down
Loading