Skip to content

Commit

Permalink
Proper fix for duplicate 'sidebar' div ID
Browse files Browse the repository at this point in the history
The initial attempt at fixing this issue [1] introduced a regression in
the persistence of the sidebar's collapse state.

Using a different ID for the toggle button or the sidebar div caused
other issues (e.g. malfunctioning hambuger menu on narrow screens, or
failure to save collapse state).

The root cause was a bug in the javascript code saving the sidebar's
state, which was referencing the toggle button's id instead of the
sidebar div's.

We now have ID 'sidebar' for the actual sidebar, and 'sidebar-btn' for
the toggle button, and the javascript has been modified accordingly.

Fixes #24976

[1]: commit b7b9145
  • Loading branch information
dregad authored and atrol committed Nov 18, 2018
1 parent 12dd350 commit b5ea0dd
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
2 changes: 1 addition & 1 deletion core/layout_api.php
Expand Up @@ -946,7 +946,7 @@ function layout_sidebar_end() {

$t_collapse_block = is_collapsed( 'sidebar' );

echo '<div id="sidebar" class="sidebar-toggle sidebar-collapse">';
echo '<div id="sidebar-btn" class="sidebar-toggle sidebar-collapse">';
if( layout_is_rtl() ) {
$t_block_icon = $t_collapse_block ? 'fa-angle-double-left' : 'fa-angle-double-right';
echo '<i data-icon2="ace-icon fa fa-angle-double-left" data-icon1="ace-icon fa fa-angle-double-right"
Expand Down
4 changes: 2 additions & 2 deletions js/common.js
Expand Up @@ -80,8 +80,8 @@ $(document).ready( function() {
SetCookie( "collapse_settings", t_cookie );
});

$('#sidebar.sidebar-toggle').on('click', function (event) {
var t_id = $(this).attr('id');
$('#sidebar-btn.sidebar-toggle').on('click', function (event) {
var t_id = $(this).closest('.sidebar').attr('id');
var t_cookie = GetCookie("collapse_settings");
if (1 == g_collapse_clear) {
t_cookie = "";
Expand Down

0 comments on commit b5ea0dd

Please sign in to comment.