From d6449ee629ce10b79aa822c80e7a42154533a4d1 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Thu, 7 Mar 2019 09:36:40 +0100 Subject: [PATCH 1/3] fix: Logo goes Home instead of toggling side menu #15482 --- .../components/sidemenu/SideMenu.test.tsx | 16 --------------- .../app/core/components/sidemenu/SideMenu.tsx | 20 +++++-------------- public/app/core/services/context_srv.ts | 10 +--------- public/app/routes/GrafanaCtrl.ts | 5 ----- 4 files changed, 6 insertions(+), 45 deletions(-) diff --git a/public/app/core/components/sidemenu/SideMenu.test.tsx b/public/app/core/components/sidemenu/SideMenu.test.tsx index f7b7df69cfc..6352833490a 100644 --- a/public/app/core/components/sidemenu/SideMenu.test.tsx +++ b/public/app/core/components/sidemenu/SideMenu.test.tsx @@ -2,7 +2,6 @@ import React from 'react'; import { shallow } from 'enzyme'; import { SideMenu } from './SideMenu'; import appEvents from '../../app_events'; -import { contextSrv } from 'app/core/services/context_srv'; jest.mock('../../app_events', () => ({ emit: jest.fn(), @@ -26,7 +25,6 @@ jest.mock('app/core/services/context_srv', () => ({ isGrafanaAdmin: false, isEditor: false, hasEditPermissionFolders: false, - toggleSideMenu: jest.fn(), }, })); @@ -54,20 +52,6 @@ describe('Render', () => { }); describe('Functions', () => { - describe('toggle side menu', () => { - const wrapper = setup(); - const instance = wrapper.instance() as SideMenu; - instance.toggleSideMenu(); - - it('should call contextSrv.toggleSideMenu', () => { - expect(contextSrv.toggleSideMenu).toHaveBeenCalled(); - }); - - it('should emit toggle sidemenu event', () => { - expect(appEvents.emit).toHaveBeenCalledWith('toggle-sidemenu'); - }); - }); - describe('toggle side menu on mobile', () => { const wrapper = setup(); const instance = wrapper.instance() as SideMenu; diff --git a/public/app/core/components/sidemenu/SideMenu.tsx b/public/app/core/components/sidemenu/SideMenu.tsx index 1428ae181f5..b0ef053f746 100644 --- a/public/app/core/components/sidemenu/SideMenu.tsx +++ b/public/app/core/components/sidemenu/SideMenu.tsx @@ -1,31 +1,21 @@ import React, { PureComponent } from 'react'; import appEvents from '../../app_events'; -import { contextSrv } from 'app/core/services/context_srv'; import TopSection from './TopSection'; import BottomSection from './BottomSection'; -import { store } from 'app/store/store'; +import config from 'app/core/config'; + +const homeUrl = config.appSubUrl || '/'; export class SideMenu extends PureComponent { - toggleSideMenu = () => { - // ignore if we just made a location change, stops hiding sidemenu on double clicks of back button - const timeSinceLocationChanged = new Date().getTime() - store.getState().location.lastUpdated; - if (timeSinceLocationChanged < 1000) { - return; - } - - contextSrv.toggleSideMenu(); - appEvents.emit('toggle-sidemenu'); - }; - toggleSideMenuSmallBreakpoint = () => { appEvents.emit('toggle-sidemenu-mobile'); }; render() { return [ -
+ Grafana -
, + ,
diff --git a/public/app/core/services/context_srv.ts b/public/app/core/services/context_srv.ts index 7bb753e6f71..b186d1fde87 100644 --- a/public/app/core/services/context_srv.ts +++ b/public/app/core/services/context_srv.ts @@ -1,7 +1,6 @@ import config from 'app/core/config'; import _ from 'lodash'; import coreModule from 'app/core/core_module'; -import store from 'app/core/store'; export class User { isGrafanaAdmin: any; @@ -29,13 +28,11 @@ export class ContextSrv { isSignedIn: any; isGrafanaAdmin: any; isEditor: any; - sidemenu: any; + sidemenu = true; sidemenuSmallBreakpoint = false; hasEditPermissionInFolders: boolean; constructor() { - this.sidemenu = store.getBool('grafana.sidemenu', true); - if (!config.bootData) { config.bootData = { user: {}, settings: {} }; } @@ -55,11 +52,6 @@ export class ContextSrv { return !!(document.visibilityState === undefined || document.visibilityState === 'visible'); } - toggleSideMenu() { - this.sidemenu = !this.sidemenu; - store.set('grafana.sidemenu', this.sidemenu); - } - hasAccessToExplore() { return (this.isEditor || config.viewersCanEdit) && config.exploreEnabled; } diff --git a/public/app/routes/GrafanaCtrl.ts b/public/app/routes/GrafanaCtrl.ts index 479c5e77f3d..45a06706cd9 100644 --- a/public/app/routes/GrafanaCtrl.ts +++ b/public/app/routes/GrafanaCtrl.ts @@ -116,11 +116,6 @@ export function grafanaAppDirective(playlistSrv, contextSrv, $timeout, $rootScop sidemenuOpen = scope.contextSrv.sidemenu; body.toggleClass('sidemenu-open', sidemenuOpen); - appEvents.on('toggle-sidemenu', () => { - sidemenuOpen = scope.contextSrv.sidemenu; - body.toggleClass('sidemenu-open'); - }); - appEvents.on('toggle-sidemenu-mobile', () => { body.toggleClass('sidemenu-open--xs'); }); From 06e9c116af053422904fdf59f63f5194faf7b591 Mon Sep 17 00:00:00 2001 From: Johannes Schill Date: Thu, 7 Mar 2019 09:54:39 +0100 Subject: [PATCH 2/3] fix: Update test snapshot --- .../sidemenu/__snapshots__/SideMenu.test.tsx.snap | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/public/app/core/components/sidemenu/__snapshots__/SideMenu.test.tsx.snap b/public/app/core/components/sidemenu/__snapshots__/SideMenu.test.tsx.snap index ec2fa845c6d..8d23cdc1565 100644 --- a/public/app/core/components/sidemenu/__snapshots__/SideMenu.test.tsx.snap +++ b/public/app/core/components/sidemenu/__snapshots__/SideMenu.test.tsx.snap @@ -2,16 +2,16 @@ exports[`Render should render component 1`] = ` Array [ -
Grafana -
, + ,
Date: Mon, 11 Mar 2019 13:18:22 +0100 Subject: [PATCH 3/3] chore: Move sidemenu out of context service and use the logic we have in the router already for hiding the sidemenu --- public/app/core/services/context_srv.ts | 1 - public/app/routes/GrafanaCtrl.ts | 17 ++--------- public/sass/components/_navbar.scss | 10 ++----- public/sass/components/_sidemenu.scss | 34 +++++++++++----------- public/sass/components/_view_states.scss | 15 ++++++++++ public/sass/pages/_explore.scss | 36 +----------------------- 6 files changed, 37 insertions(+), 76 deletions(-) diff --git a/public/app/core/services/context_srv.ts b/public/app/core/services/context_srv.ts index b186d1fde87..e3b10f129d1 100644 --- a/public/app/core/services/context_srv.ts +++ b/public/app/core/services/context_srv.ts @@ -28,7 +28,6 @@ export class ContextSrv { isSignedIn: any; isGrafanaAdmin: any; isEditor: any; - sidemenu = true; sidemenuSmallBreakpoint = false; hasEditPermissionInFolders: boolean; diff --git a/public/app/routes/GrafanaCtrl.ts b/public/app/routes/GrafanaCtrl.ts index 45a06706cd9..a37222091d0 100644 --- a/public/app/routes/GrafanaCtrl.ts +++ b/public/app/routes/GrafanaCtrl.ts @@ -75,27 +75,22 @@ export class GrafanaCtrl { } } -function setViewModeBodyClass(body: JQuery, mode: KioskUrlValue, sidemenuOpen: boolean) { +function setViewModeBodyClass(body: JQuery, mode: KioskUrlValue) { body.removeClass('view-mode--tv'); body.removeClass('view-mode--kiosk'); body.removeClass('view-mode--inactive'); switch (mode) { case 'tv': { - body.removeClass('sidemenu-open'); body.addClass('view-mode--tv'); break; } // 1 & true for legacy states case '1': case true: { - body.removeClass('sidemenu-open'); body.addClass('view-mode--kiosk'); break; } - default: { - body.toggleClass('sidemenu-open', sidemenuOpen); - } } } @@ -105,7 +100,6 @@ export function grafanaAppDirective(playlistSrv, contextSrv, $timeout, $rootScop restrict: 'E', controller: GrafanaCtrl, link: (scope, elem) => { - let sidemenuOpen; const body = $('body'); // see https://github.com/zenorocha/clipboard.js/issues/155 @@ -113,9 +107,6 @@ export function grafanaAppDirective(playlistSrv, contextSrv, $timeout, $rootScop $('.preloader').remove(); - sidemenuOpen = scope.contextSrv.sidemenu; - body.toggleClass('sidemenu-open', sidemenuOpen); - appEvents.on('toggle-sidemenu-mobile', () => { body.toggleClass('sidemenu-open--xs'); }); @@ -158,7 +149,7 @@ export function grafanaAppDirective(playlistSrv, contextSrv, $timeout, $rootScop $('#tooltip, .tooltip').remove(); // check for kiosk url param - setViewModeBodyClass(body, data.params.kiosk, sidemenuOpen); + setViewModeBodyClass(body, data.params.kiosk); // close all drops for (const drop of Drop.drops) { @@ -193,7 +184,7 @@ export function grafanaAppDirective(playlistSrv, contextSrv, $timeout, $rootScop } $timeout(() => $location.search(search)); - setViewModeBodyClass(body, search.kiosk, sidemenuOpen); + setViewModeBodyClass(body, search.kiosk); }); // handle in active view state class @@ -213,7 +204,6 @@ export function grafanaAppDirective(playlistSrv, contextSrv, $timeout, $rootScop if (new Date().getTime() - lastActivity > inActiveTimeLimit) { activeUser = false; body.addClass('view-mode--inactive'); - body.removeClass('sidemenu-open'); } } @@ -222,7 +212,6 @@ export function grafanaAppDirective(playlistSrv, contextSrv, $timeout, $rootScop if (!activeUser) { activeUser = true; body.removeClass('view-mode--inactive'); - body.toggleClass('sidemenu-open', sidemenuOpen); } } diff --git a/public/sass/components/_navbar.scss b/public/sass/components/_navbar.scss index a86b8c450a0..eef101c98b4 100644 --- a/public/sass/components/_navbar.scss +++ b/public/sass/components/_navbar.scss @@ -157,14 +157,8 @@ @include media-breakpoint-up(sm) { .navbar { - padding-left: 60px; - } - - .sidemenu-open { - .navbar { - padding-left: 25px; - margin-left: 0; - } + padding-left: 20px; + margin-left: 0; } .navbar-page-btn { diff --git a/public/sass/components/_sidemenu.scss b/public/sass/components/_sidemenu.scss index f30bdb5c79e..c16e037c2ad 100644 --- a/public/sass/components/_sidemenu.scss +++ b/public/sass/components/_sidemenu.scss @@ -16,6 +16,14 @@ .sidemenu__close { display: none; } + + @include media-breakpoint-up(sm) { + background: $side-menu-bg; + height: auto; + box-shadow: $side-menu-shadow; + position: relative; + z-index: $zindex-sidemenu; + } } // body class that hides sidemenu @@ -25,32 +33,22 @@ } } -@include media-breakpoint-up(sm) { - .sidemenu-open { - .sidemenu { - background: $side-menu-bg; - height: auto; - box-shadow: $side-menu-shadow; - position: relative; - z-index: $zindex-sidemenu; - } - - .sidemenu__top, - .sidemenu__bottom { - display: block; - } - } -} - .sidemenu__top { padding-top: 3rem; flex-grow: 1; - display: none; } .sidemenu__bottom { padding-bottom: $spacer; +} + +.sidemenu__top, +.sidemenu__bottom { display: none; + + @include media-breakpoint-up(sm) { + display: block; + } } .sidemenu-item { diff --git a/public/sass/components/_view_states.scss b/public/sass/components/_view_states.scss index b92bd596193..e1ca3d44f83 100644 --- a/public/sass/components/_view_states.scss +++ b/public/sass/components/_view_states.scss @@ -29,6 +29,21 @@ .view-mode--tv { @extend .view-mode--inactive; + .sidemenu { + position: fixed; + background-color: transparent; + box-shadow: none; + + .sidemenu__top, + .sidemenu__bottom { + display: none; + } + } + + .navbar { + padding-left: $side-menu-width; + } + .submenu-controls { display: none; } diff --git a/public/sass/pages/_explore.scss b/public/sass/pages/_explore.scss index 90579ff67ad..0358adb9787 100644 --- a/public/sass/pages/_explore.scss +++ b/public/sass/pages/_explore.scss @@ -25,20 +25,13 @@ } } -.sidemenu-open { - .explore-toolbar-header { - padding: 0; - margin-left: 0; - } -} - .explore-toolbar { background: inherit; display: flex; flex-flow: row wrap; justify-content: flex-start; height: auto; - padding: 0px $dashboard-padding 0 25px; + padding: 0 $dashboard-padding; border-bottom: 1px solid #0000; transition-duration: 0.35s; transition-timing-function: ease-in-out; @@ -72,11 +65,6 @@ font-size: 18px; min-height: 55px; line-height: 55px; - justify-content: space-between; - margin-left: $panel-margin * 3; -} - -.explore-toolbar-header { justify-content: space-between; align-items: center; } @@ -134,20 +122,6 @@ } @media only screen and (max-width: 803px) { - .sidemenu-open { - .explore-toolbar-header-title { - .navbar-page-btn { - margin-left: 0; - } - } - } - - .explore-toolbar-header-title { - .navbar-page-btn { - margin-left: $dashboard-padding; - } - } - .btn-title { display: none; } @@ -161,14 +135,6 @@ } @media only screen and (max-width: 544px) { - .sidemenu-open { - .explore-toolbar-header-title { - .navbar-page-btn { - margin-left: $dashboard-padding; - } - } - } - .explore-toolbar-header-title { .navbar-page-btn { margin-left: $dashboard-padding;