Since #1850164: Default state of toolbar should show menu tray in non-narrow viewports the toolbar always stays open. This is non-intuitive. If I close the toolbar, I do not expect it to be open again on the next page refresh.

Both this issue and the aforementioned one can be accomplished at the same time. The toolbar can default to open, and still remember its open/closed state.

Files: 

Comments

twistor created an issue. See original summary.

twistor’s picture

Title: Make the toolbar remember its open/closed state. » Make the toolbar remember its open/closed state when in the horizontal position.
Status: Active » Needs review
FileSize
1.48 KB
twistor’s picture

This one should actually work.

I don't really like using the tri-state variable, (null, false, string) but it works for now. It can be polished up if agreement is reached.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mathieuhelie’s picture

Status: Needs review » Reviewed & tested by the community

Patch tests correctly on my end. I see what you mean about using a 'false' state to represent the closed toolbar, it's not obvious what the behavior of the code is, or why we are saving false instead of unsetting the localstorage state. We could use a string literal such as "active-tab-none" and modify the view's render logic to check for that literal.

I will try to rework the patch along those lines, but I approve 2667396-3.patch.

Cottser’s picture

Issue tags: +JavaScript
esod’s picture

The patch works for me. Adding testing for 8.2.x and 8.1.x

Cottser’s picture

Should we consider adding another property to the model to store a boolean rather than complicating activeTab?

droplet’s picture

Oh. Seems like this issue would make my hard working doesn't work anymore and we can't fix the issue forever:
#2542050: Toolbar implementation creates super annoying re-rendering.

Unless we do this:
#2696023: Save Users' Toolbar State config to serverside

I think we should consider above 2 issues before this commit. It creates more noise and unable to revert in the future.

Cottser’s picture

Status: Reviewed & tested by the community » Needs review
mathieuhelie’s picture

I agree with #8, whether or not the bottom tray is open is a different state than what the active tab is.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

thamas’s picture

Status: Needs review » Needs work

This issue could resolve a big (UX) pain of me about Drupal 8!

While the latest patch just works for me with the latest Drupal version of this time (8.2.3) I just mark this as Needs Work because of the comments in #9 and #8.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.