Commit d9f9ad9d authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'fix-sidebar' into 'master'

Fix broken left sidebar navigation

Sidebar navigation is broken and it's introduced in !6627. Sidebar pin and hamburger icons were `a` tag before with `#` link on them. That was the main reason why page jumps to top. !6627 was trying to prevent default action but it interferes with the navigation links. I don't want to tweak event listeners for sidebar, instead I just replaced `a` elements with `div` because I think there is no point to have them as `a`.

/cc @dzaporozhets @jschatz1 

PS: This MR basically reverts !6627. I don't want to add another revert commit instead I reverted it in my editor and committed.

See merge request !6648
parents 061dd18b 729cb3b3
...@@ -34,8 +34,8 @@ ...@@ -34,8 +34,8 @@
$(pageSelector).hasClass(expandedPageClass) $(pageSelector).hasClass(expandedPageClass)
); );
$(document) $(document)
.on('click', sidebarToggleSelector, (e) => this.toggleSidebar(e)) .on('click', sidebarToggleSelector, () => this.toggleSidebar())
.on('click', pinnedToggleSelector, (e) => this.togglePinnedState(e)) .on('click', pinnedToggleSelector, () => this.togglePinnedState())
.on('click', 'html, body', (e) => this.handleClickEvent(e)) .on('click', 'html, body', (e) => this.handleClickEvent(e))
.on('page:change', () => this.renderState()); .on('page:change', () => this.renderState());
this.renderState(); this.renderState();
...@@ -47,19 +47,17 @@ ...@@ -47,19 +47,17 @@
const targetIsToggle = $target.closest(sidebarToggleSelector).length > 0; const targetIsToggle = $target.closest(sidebarToggleSelector).length > 0;
const targetIsSidebar = $target.closest(sidebarWrapperSelector).length > 0; const targetIsSidebar = $target.closest(sidebarWrapperSelector).length > 0;
if (!targetIsToggle && (!targetIsSidebar || $target.closest('a'))) { if (!targetIsToggle && (!targetIsSidebar || $target.closest('a'))) {
this.toggleSidebar(e); this.toggleSidebar();
} }
} }
} }
toggleSidebar(e) { toggleSidebar() {
e.preventDefault();
this.isExpanded = !this.isExpanded; this.isExpanded = !this.isExpanded;
this.renderState(); this.renderState();
} }
togglePinnedState(e) { togglePinnedState() {
e.preventDefault();
this.isPinned = !this.isPinned; this.isPinned = !this.isPinned;
if (!this.isPinned) { if (!this.isPinned) {
this.isExpanded = false; this.isExpanded = false;
......
...@@ -142,6 +142,7 @@ ...@@ -142,6 +142,7 @@
transition-duration: .3s; transition-duration: .3s;
position: absolute; position: absolute;
top: 0; top: 0;
cursor: pointer;
&:hover, &:hover,
&:focus { &:focus {
......
.page-with-sidebar{ class: "#{page_sidebar_class} #{page_gutter_class}" } .page-with-sidebar{ class: "#{page_sidebar_class} #{page_gutter_class}" }
.sidebar-wrapper.nicescroll .sidebar-wrapper.nicescroll
.sidebar-action-buttons .sidebar-action-buttons
= link_to '#', class: 'nav-header-btn toggle-nav-collapse', title: "Open/Close" do .nav-header-btn.toggle-nav-collapse{ title: "Open/Close" }
%span.sr-only Toggle navigation %span.sr-only Toggle navigation
= icon('bars') = icon('bars')
= link_to '#', class: "nav-header-btn pin-nav-btn has-tooltip #{'is-active' if pinned_nav?} js-nav-pin", title: pinned_nav? ? "Unpin navigation" : "Pin Navigation", data: {placement: 'right', container: 'body'} do
%div{ class: "nav-header-btn pin-nav-btn has-tooltip #{'is-active' if pinned_nav?} js-nav-pin", title: pinned_nav? ? "Unpin navigation" : "Pin Navigation", data: { placement: 'right', container: 'body' } }
%span.sr-only Toggle navigation pinning %span.sr-only Toggle navigation pinning
= icon('fw thumb-tack') = icon('fw thumb-tack')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment