Admin-side navbar collapse #3166

seand7565 · 2019-04-09T00:39:15Z

Description
Makes the admin-side navbar collapsible by clicking a collapse icon at the top of the page.

Additional functionality:

The navbar defaults to closed on iPad sized screens or smaller.
When the navbar is collapsed or opened, a cookie is stored that remembers your preference - on the next page load, the navbar will be in the same state as it was on the previous page.
When the navbar is closed, a tooltip is set for the navbar icons which - on hover- displays the name of the button you have selected. If you prefer a closed navbar, but don't know what the icons mean, you can still find your way around.

@jacobherrington and @tvdeyen did most of the heavy lifting here, I just cleaned it up and added a few things.

ref #2961 - should close this issue if everything looks good!

Checklist:

I have followed Pull Request guidelines
I have added a detailed description into each commit message
I have updated Guides and README accordingly to this change (if needed)
I have added tests to cover this change (if needed)

seand7565 · 2019-04-10T22:47:42Z

Got some tests to fix here - capybara is trying to click on nested navbar links, but those are hidden when the navbar is collapsed. The navbar defaults to closed on small screens, which is the problem for these tests.

The two solutions I have are to increase capybaras window size, or just click the navbar-toggle button to open the navbar before running the tests. I'll pop something in soon to get the tests to pass.

tvdeyen · 2019-04-10T23:08:04Z

Thanks, this looks great! Regarding the specs: My vote is to raise the capybara window size.

aitbw · 2019-04-11T11:02:09Z

My vote too goes to raise Capybara window size. This looks very good, @seand7565! Thanks a lot 🙌

One small thing: is it possible you could rebase this PR? That'd would help us avoid the merge commit in here

vzqzac

I like this feature, just that the commits need some love 😅

seand7565 · 2019-04-11T16:51:23Z

(Mostly) passing now! @aitbw The screen size actually wasn't the issue here like I thought, I used the wrong operator in the javascript file determining the default screen size (greater than rather than less than). D'oh!

Not sure why the mysql rails 5.1 tests failed - and on the flash too, which wasn't touched in this commit. I'll see if I can find out why it failed!

Thanks all for the feedback! ❤️ I rebased and squashed some of the smaller changes - not sure how far you folks like PRs squashed. Should this all be one commit?

seand7565 · 2019-05-03T12:48:49Z

The test that was previously failing here appears to be passing now after a rebase 🤷‍♂

spaghetticode · 2019-06-03T13:47:13Z

@seand7565 Thanks for the PR 👏

I see it now needs some rebasing, some asset files are out-of-sync, can you please update them? Thank you!

…for the class to be set on body instantly, rather than having to wait for page load

…till being clickable

seand7565 · 2019-06-07T01:26:19Z

This doesn't play well with the new admin styling - will have to make some changes to get it back in line. Give me a few days and I'll have it done!

backend/app/assets/javascripts/spree/backend/components/admin_nav.js

seand7565 · 2019-06-11T01:43:36Z

Should be good to go now!

tvdeyen

Thanks 🎉

jacobherrington

Thanks for carrying this one to the finish line @seand7565! 🏃

elia · 2019-06-17T12:34:43Z

backend/app/assets/javascripts/spree/backend/components/admin_nav.js

+  if (window.screen.width <= 1024 && !document.cookie.includes("admin_nav_hidden")) {
+    // Set default nav to collapse on small screens - but don't override user preference
+    document.body.classList.add("admin-nav-hidden");
+    document.cookie = "admin_nav_hidden=true; expires=Fri, 31 Dec 9999 23:59:59 GMT";


Any reason not to use LocalStorage instead?
Browser support is pretty wide https://caniuse.com/#feat=namevalue-storage
I would also suggest namespacing the key with solidus. or solidus/, e.g. window.localStorage.get('solidus.minimize-admin-nav')

I believe I was initially using LocalStorage, but there was some feedback on that decision from either @tvdeyen or @seand7565. I do not remember it 🤦‍♂

kennyadsl · 2019-06-18T10:02:45Z

I'm ok with this but I think it will need some extra design iterations after the merge. If you agree I'd merge it after v2.9 release which already contains a lot of backend changes.

mfrecchiami · 2019-07-19T16:36:27Z

Hi @seand7565 and everyone!

I tried to improve a little bit more this new feature, moving the collapse icon and leaving the Solidus mark always visible.

This is the first version 👇

Then I tried to move to the below the collapse button 👇

Which one do you think that could be the best solution?

Note: when you log in with a user whit role of Admin, there is a select for change the language, at the bottom of the sidebar; as you can see by the screenshots above, I choose to hide it when the navbar is collapsed

seand7565 · 2019-07-19T18:55:43Z

That looks fantastic - thanks @mfrecchiami ! I like the button being inside the collapsible panel, so it doesn't affect the layout of the rest of the page.

mfrecchiami · 2019-09-06T08:27:36Z

Hi @seand7565 !

I did my job starting from your repo 💪
I've made my branch, did the rebase with master and added my commits
I believed to find your repo in the compare step, but it wasn't 😞 (still don't know why)
So I made another PR related to yours: #3322

Do you want to do it in a different way? Let me know!

seand7565 · 2019-09-06T13:06:55Z

@mfrecchiami No, that's perfect - thanks! I'll close this PR in favor of yours.

aitbw mentioned this pull request Apr 11, 2019

Increase Capybara window width size #3171

Merged

4 tasks

vzqzac suggested changes Apr 11, 2019

View reviewed changes

seand7565 force-pushed the nav-collapse branch from 77f8263 to 36dc4aa Compare April 11, 2019 16:26

seand7565 force-pushed the nav-collapse branch from 36dc4aa to ec1d1f2 Compare May 3, 2019 12:03

jacobherrington and others added 12 commits June 3, 2019 12:55

Add a button to hide nav

8b25c0d

Fix teaspoon tests

8e7bfb3

Add transistions and simplify JS

0c08c74

Clean up SCSS

eb6ea4d

Incorporate styling tweaks

7042882

Add a session variable to remember nav state

1c1469b

Rebuild collapsible nav

8e8e2d8

Style changes for navbar collapse

d655657

Switching to cookies for admin-nav-hidden state storage. This allows …

5c6c5fe

…for the class to be set on body instantly, rather than having to wait for page load

Defaulting to closed on small screens

5bf10af

Using display:none; for nav menu text to avoid hidden elements from s…

2c5a8cd

…till being clickable

Using tooltips for admin navbar when it's hidden

419c6c6

seand7565 force-pushed the nav-collapse branch from ec1d1f2 to 3ab8425 Compare June 10, 2019 18:07

houndci-bot reviewed Jun 10, 2019

View reviewed changes

seand7565 force-pushed the nav-collapse branch from 3ab8425 to 1f08981 Compare June 10, 2019 23:50

houndci-bot reviewed Jun 10, 2019

View reviewed changes

backend/app/assets/javascripts/spree/backend/components/admin_nav.js Outdated Show resolved Hide resolved

seand7565 force-pushed the nav-collapse branch from 1f08981 to 24372bb Compare June 11, 2019 00:28

Formatting nav-collapse to work with new admin layout

97db7f1

seand7565 force-pushed the nav-collapse branch from 24372bb to 97db7f1 Compare June 11, 2019 01:08

tvdeyen approved these changes Jun 11, 2019

View reviewed changes

jacobherrington approved these changes Jun 14, 2019

View reviewed changes

elia requested changes Jun 17, 2019

View reviewed changes

mfrecchiami mentioned this pull request Aug 30, 2019

Style collapsing sidebar #3322

Merged

seand7565 closed this Sep 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Admin-side navbar collapse #3166

Admin-side navbar collapse #3166

seand7565 commented Apr 9, 2019 •

edited

Loading

seand7565 commented Apr 10, 2019

tvdeyen commented Apr 10, 2019

aitbw commented Apr 11, 2019

vzqzac left a comment

seand7565 commented Apr 11, 2019

seand7565 commented May 3, 2019

spaghetticode commented Jun 3, 2019

seand7565 commented Jun 7, 2019

seand7565 commented Jun 11, 2019

tvdeyen left a comment

jacobherrington left a comment

elia Jun 17, 2019

jacobherrington Jun 17, 2019

kennyadsl commented Jun 18, 2019

mfrecchiami commented Jul 19, 2019 •

edited

Loading

seand7565 commented Jul 19, 2019

mfrecchiami commented Sep 6, 2019

seand7565 commented Sep 6, 2019

Admin-side navbar collapse #3166

Admin-side navbar collapse #3166

Conversation

seand7565 commented Apr 9, 2019 • edited Loading

seand7565 commented Apr 10, 2019

tvdeyen commented Apr 10, 2019

aitbw commented Apr 11, 2019

vzqzac left a comment

Choose a reason for hiding this comment

seand7565 commented Apr 11, 2019

seand7565 commented May 3, 2019

spaghetticode commented Jun 3, 2019

seand7565 commented Jun 7, 2019

seand7565 commented Jun 11, 2019

tvdeyen left a comment

Choose a reason for hiding this comment

jacobherrington left a comment

Choose a reason for hiding this comment

elia Jun 17, 2019

Choose a reason for hiding this comment

jacobherrington Jun 17, 2019

Choose a reason for hiding this comment

kennyadsl commented Jun 18, 2019

mfrecchiami commented Jul 19, 2019 • edited Loading

seand7565 commented Jul 19, 2019

mfrecchiami commented Sep 6, 2019

seand7565 commented Sep 6, 2019

seand7565 commented Apr 9, 2019 •

edited

Loading

mfrecchiami commented Jul 19, 2019 •

edited

Loading