Browse Source

feat(settings): Toggle settings nav for smaller browser widths (#15946)

* feat(settings): Toggle settings nav for smaller browser widths
* ref(ui): Reorder styles, remove unused styles because of changed markup
* ref(ui): Refactor hamburger icon, new theme colors
* ref(ui): Add more typescript types
* ref(ui): Replace style values with variables from theme
* fix(ui): Calculate settings navigation height
* feat(ui): Close navigation on overlay click
* fix(ui): Do not show hamburger on settings index page
* fix(ui): Correct navigation position, ios scroll lock the body
* ref(ui): Move state init out of constructor, remove fragment
* ref(ui): Move scroll lock to css class
* feat(ui): Make hamburger clickable area bigger
* test(ui): Add mobile menu test coverage
* feat(ui): Improve the a11y of hamburger button
* test(ui): Add visual snapshot of the mobile menu
* ref(ui): Remove construvtor
* fix(ui): Conditional scrollTo
* ref(ui): Move state comments

Co-authored-by: Evan Purkhiser <evanpurkhiser@gmail.com>
Co-authored-by: Matej Minar <matej.minar@sentry.io>
Co-authored-by: Mark Story <mark@sentry.io>
John Manhart 4 years ago
parent
commit
b8227d06b2

+ 4 - 1
src/sentry/static/sentry/app/utils/theme.tsx

@@ -260,6 +260,9 @@ const theme = {
     },
 
     globalSelectionHeader: 1009,
+
+    settingsSidebarNavMask: 1017,
+    settingsSidebarNav: 1018,
     sidebarPanel: 1019,
     sidebar: 1020,
     orgAndUserMenu: 1030,
@@ -295,7 +298,7 @@ const theme = {
     maxCrumbWidth: '240px',
 
     containerWidth: '1440px',
-    headerHeight: '115px',
+    headerHeight: '69px',
     sidebarWidth: '220px',
   },
 

+ 2 - 0
src/sentry/static/sentry/app/views/settings/components/settingsBreadcrumb/crumb.jsx

@@ -10,6 +10,8 @@ const Crumb = styled('div')`
   color: ${p => p.theme.gray600};
   padding-right: ${space(1)};
   cursor: pointer;
+  white-space: nowrap;
+
   > span {
     transition: 0.1s all ease;
   }

+ 1 - 0
src/sentry/static/sentry/app/views/settings/components/settingsHeader.tsx

@@ -15,6 +15,7 @@ const SettingsHeader = styled('div')`
   padding: ${space(3)} ${space(4)};
   border-bottom: 1px solid ${p => p.theme.borderLight};
   background: #fff;
+  height: ${p => p.theme.settings.headerHeight};
 `;
 
 export default SettingsHeader;

+ 144 - 36
src/sentry/static/sentry/app/views/settings/components/settingsLayout.tsx

@@ -1,9 +1,14 @@
 import {RouteComponentProps} from 'react-router/lib/Router';
+import {browserHistory} from 'react-router';
 import PropTypes from 'prop-types';
 import React from 'react';
 import styled from '@emotion/styled';
 
+import {t} from 'app/locale';
 import space from 'app/styles/space';
+import Button from 'app/components/button';
+import {slideInLeft, fadeIn} from 'app/styles/animations';
+import {IconClose, IconMenu} from 'app/icons';
 
 import SettingsBreadcrumb from './settingsBreadcrumb';
 import SettingsHeader from './settingsHeader';
@@ -14,18 +19,86 @@ type Props = {
   children: React.ReactNode;
 } & RouteComponentProps<{}, {}>;
 
-function SettingsLayout(props: Props) {
-  const {params, routes, route, router, renderNavigation, children} = props;
+type State = {
+  /**
+   * This is used when the screen is small enough that the navigation should
+   * be hidden. On large screens this state will end up unused.
+   */
+  navVisible: boolean;
+  /**
+   * Offset mobile settings navigation by the height of main navigation,
+   * settings breadcrumbs and optional warnings.
+   */
+  navOffsetTop: number;
+};
+
+class SettingsLayout extends React.Component<Props, State> {
+  static propTypes = {
+    renderNavigation: PropTypes.func,
+    route: PropTypes.object,
+    router: PropTypes.object,
+    routes: PropTypes.array,
+  };
+
+  state = {
+    navVisible: false,
+    navOffsetTop: 0,
+  };
+
+  componentDidMount() {
+    // Close the navigation when navigating.
+    this.unlisten = browserHistory.listen(() => this.toggleNav(false));
+  }
+
+  componentWillUnmount() {
+    this.unlisten();
+  }
+
+  unlisten!: () => void;
+  headerRef = React.createRef<HTMLDivElement>();
+
+  toggleNav(navVisible: boolean) {
+    // when the navigation is opened, body should be scroll-locked
+    this.toggleBodyScrollLock(navVisible);
+
+    this.setState({
+      navOffsetTop: this.headerRef.current?.getBoundingClientRect().bottom ?? 0,
+      navVisible,
+    });
+  }
+
+  toggleBodyScrollLock(lock: boolean) {
+    const bodyElement = document.getElementsByTagName('body')[0];
+
+    if (window.scrollTo) {
+      window.scrollTo(0, 0);
+    }
+    bodyElement.classList[lock ? 'add' : 'remove']('scroll-lock');
+  }
 
-  // We want child's view's props
-  const childProps = children && React.isValidElement(children) ? children.props : props;
-  const childRoutes = childProps.routes || routes || [];
-  const childRoute = childProps.route || route || {};
-  return (
-    <React.Fragment>
+  render() {
+    const {params, routes, route, router, renderNavigation, children} = this.props;
+    const {navVisible, navOffsetTop} = this.state;
+
+    // We want child's view's props
+    const childProps =
+      children && React.isValidElement(children) ? children.props : this.props;
+    const childRoutes = childProps.routes || routes || [];
+    const childRoute = childProps.route || route || {};
+    const shouldRenderNavigation = typeof renderNavigation === 'function';
+
+    return (
       <SettingsColumn>
-        <SettingsHeader>
+        <SettingsHeader ref={this.headerRef}>
           <HeaderContent>
+            {shouldRenderNavigation && (
+              <NavMenuToggle
+                priority="link"
+                label={t('Open the menu')}
+                icon={navVisible ? <IconClose aria-hidden /> : <IconMenu aria-hidden />}
+                onClick={() => this.toggleNav(!navVisible)}
+              />
+            )}
             <StyledSettingsBreadcrumb
               params={params}
               routes={childRoutes}
@@ -36,55 +109,90 @@ function SettingsLayout(props: Props) {
         </SettingsHeader>
 
         <MaxWidthContainer>
-          {typeof renderNavigation === 'function' && (
-            <SidebarWrapper>{renderNavigation()}</SidebarWrapper>
+          {shouldRenderNavigation && (
+            <SidebarWrapper isVisible={navVisible} offsetTop={navOffsetTop}>
+              {renderNavigation!()}
+            </SidebarWrapper>
           )}
+          <NavMask isVisible={navVisible} onClick={() => this.toggleNav(false)} />
           <Content>{children}</Content>
         </MaxWidthContainer>
       </SettingsColumn>
-    </React.Fragment>
-  );
+    );
+  }
 }
 
-SettingsLayout.propTypes = {
-  renderNavigation: PropTypes.func,
-  route: PropTypes.object,
-  router: PropTypes.object,
-  routes: PropTypes.array,
-};
+const SettingsColumn = styled('div')`
+  display: flex;
+  flex-direction: column;
+  flex: 1; /* so this stretches vertically so that footer is fixed at bottom */
+  min-width: 0; /* fixes problem when child content stretches beyond layout width */
+  footer {
+    margin-top: 0;
+  }
+`;
+
+const HeaderContent = styled('div')`
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+`;
+
+const NavMenuToggle = styled(Button)`
+  display: none;
+  margin: -${space(1)} ${space(1)} -${space(1)} -${space(1)};
+  padding: ${space(1)};
+  color: ${p => p.theme.gray600};
+  &:hover,
+  &:focus,
+  &:active {
+    color: ${p => p.theme.gray800};
+  }
+  @media (max-width: ${p => p.theme.breakpoints[0]}) {
+    display: block;
+  }
+`;
+
+const StyledSettingsBreadcrumb = styled(SettingsBreadcrumb)`
+  flex: 1;
+`;
 
 const MaxWidthContainer = styled('div')`
   display: flex;
   max-width: ${p => p.theme.settings.containerWidth};
-  min-width: 600px; /* for small screen sizes, we need a min width to make it semi digestible */
   flex: 1;
 `;
 
-const SidebarWrapper = styled('div')`
+const SidebarWrapper = styled('div')<{isVisible: boolean; offsetTop: number}>`
   flex-shrink: 0;
   width: ${p => p.theme.settings.sidebarWidth};
   background: ${p => p.theme.white};
   border-right: 1px solid ${p => p.theme.borderLight};
   padding: ${space(4)};
   padding-right: ${space(2)};
-`;
-
-const HeaderContent = styled('div')`
-  display: flex;
-  align-items: center;
-`;
 
-const StyledSettingsBreadcrumb = styled(SettingsBreadcrumb)`
-  flex: 1;
+  @media (max-width: ${p => p.theme.breakpoints[0]}) {
+    display: ${p => (p.isVisible ? 'block' : 'none')};
+    position: fixed;
+    top: ${p => p.offsetTop}px;
+    bottom: 0;
+    overflow-y: auto;
+    animation: ${slideInLeft} 100ms ease-in-out;
+    z-index: ${p => p.theme.zIndex.settingsSidebarNav};
+    box-shadow: ${p => p.theme.dropShadowHeavy};
+  }
 `;
 
-const SettingsColumn = styled('div')`
-  display: flex;
-  flex-direction: column;
-  flex: 1; /* so this stretches vertically so that footer is fixed at bottom */
-  min-width: 0; /* fixes problem when child content stretches beyond layout width */
-  footer {
-    margin-top: 0;
+const NavMask = styled('div')<{isVisible: boolean}>`
+  display: none;
+  @media (max-width: ${p => p.theme.breakpoints[0]}) {
+    display: ${p => (p.isVisible ? 'block' : 'none')};
+    background: rgba(0, 0, 0, 0.35);
+    height: 100%;
+    width: 100%;
+    position: absolute;
+    z-index: ${p => p.theme.zIndex.settingsSidebarNavMask};
+    animation: ${fadeIn} 250ms ease-in-out;
   }
 `;
 

+ 6 - 0
src/sentry/static/sentry/less/layout.less

@@ -32,6 +32,12 @@ body {
       display: none;
     }
   }
+
+  &.scroll-lock {
+    overflow: hidden;
+    position: fixed;
+    width: 100%;
+  }
 }
 
 #blk_router {

+ 14 - 0
tests/acceptance/test_project_general_settings.py

@@ -19,3 +19,17 @@ class ProjectGeneralSettingsTest(AcceptanceTestCase):
         self.browser.get(path)
         self.browser.wait_until_not(".loading-indicator")
         self.browser.snapshot("project settings - general settings")
+
+    def test_mobile_menu(self):
+        path = u"/{}/{}/settings/".format(self.org.slug, self.project.slug)
+        self.browser.get(path)
+        self.browser.wait_until_not(".loading-indicator")
+
+        try:
+            self.browser.click('[aria-label="Open the menu"]')
+            self.browser.wait_until("body.scroll-lock")
+
+        except Exception:
+            pass
+
+        self.browser.snapshot("project settings - mobile menu")

+ 6 - 2
tests/js/spec/components/__snapshots__/dropdownAutoCompleteMenu.spec.jsx.snap

@@ -98,7 +98,9 @@ exports[`DropdownAutoCompleteMenu renders with a group 1`] = `
                         "30px",
                         ";border-bottom:1px solid ",
                         [Function],
-                        ";background:#fff;",
+                        ";background:#fff;height:",
+                        [Function],
+                        ";",
                       ],
                       "defaultProps": undefined,
                       "displayName": "SettingsHeader",
@@ -337,7 +339,9 @@ exports[`DropdownAutoCompleteMenu renders without a group 1`] = `
                         "30px",
                         ";border-bottom:1px solid ",
                         [Function],
-                        ";background:#fff;",
+                        ";background:#fff;height:",
+                        [Function],
+                        ";",
                       ],
                       "defaultProps": undefined,
                       "displayName": "SettingsHeader",

+ 36 - 18
tests/js/spec/components/__snapshots__/settingsLayout.spec.jsx.snap

@@ -1,22 +1,40 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
 exports[`SettingsLayout renders 1`] = `
-<Fragment>
-  <SettingsColumn>
-    <SettingsHeader>
-      <HeaderContent>
-        <StyledSettingsBreadcrumb
-          route={Object {}}
-          routes={Array []}
-        />
-        <StyledSettingsSearch
-          routes={Array []}
-        />
-      </HeaderContent>
-    </SettingsHeader>
-    <MaxWidthContainer>
-      <Content />
-    </MaxWidthContainer>
-  </SettingsColumn>
-</Fragment>
+<SettingsColumn>
+  <SettingsHeader>
+    <HeaderContent>
+      <StyledSettingsBreadcrumb
+        route={Object {}}
+        routes={Array []}
+      />
+      <StyledSettingsSearch
+        router={
+          Object {
+            "createHref": [MockFunction],
+            "go": [MockFunction],
+            "goBack": [MockFunction],
+            "goForward": [MockFunction],
+            "isActive": [MockFunction],
+            "listen": [MockFunction],
+            "location": Object {
+              "query": Object {},
+            },
+            "push": [MockFunction],
+            "replace": [MockFunction],
+            "setRouteLeaveHook": [MockFunction],
+          }
+        }
+        routes={Array []}
+      />
+    </HeaderContent>
+  </SettingsHeader>
+  <MaxWidthContainer>
+    <NavMask
+      isVisible={false}
+      onClick={[Function]}
+    />
+    <Content />
+  </MaxWidthContainer>
+</SettingsColumn>
 `;

+ 3 - 1
tests/js/spec/components/events/interfaces/breadcrumbs/__snapshots__/filter.spec.tsx.snap

@@ -631,7 +631,7 @@ exports[`Filter default render 1`] = `
                     "red500": "#AC1025",
                     "settings": Object {
                       "containerWidth": "1440px",
-                      "headerHeight": "115px",
+                      "headerHeight": "69px",
                       "maxCrumbWidth": "240px",
                       "sidebarWidth": "220px",
                     },
@@ -696,6 +696,8 @@ exports[`Filter default render 1`] = `
                       "modal": 10000,
                       "orgAndUserMenu": 1030,
                       "sentryErrorEmbed": 1090,
+                      "settingsSidebarNav": 1018,
+                      "settingsSidebarNavMask": 1017,
                       "sidebar": 1020,
                       "sidebarPanel": 1019,
                       "toast": 10001,

+ 29 - 2
tests/js/spec/components/settingsLayout.spec.jsx

@@ -33,7 +33,9 @@ describe('SettingsLayout', function() {
   });
 
   it('renders', function() {
-    const wrapper = shallow(<SettingsLayout route={{}} routes={[]} />);
+    const wrapper = shallow(
+      <SettingsLayout router={TestStubs.router()} route={{}} routes={[]} />
+    );
 
     expect(wrapper).toMatchSnapshot();
   });
@@ -41,9 +43,34 @@ describe('SettingsLayout', function() {
   it('can render navigation', function() {
     const Navigation = () => <div>Navigation</div>;
     const wrapper = shallow(
-      <SettingsLayout route={{}} routes={[]} renderNavigation={() => <Navigation />} />
+      <SettingsLayout
+        router={TestStubs.router()}
+        route={{}}
+        routes={[]}
+        renderNavigation={() => <Navigation />}
+      />
     );
 
     expect(wrapper.find('Navigation')).toHaveLength(1);
   });
+
+  it('can toggle mobile navigation', function() {
+    const Navigation = () => <div>Navigation</div>;
+    const wrapper = shallow(
+      <SettingsLayout
+        router={TestStubs.router()}
+        route={{}}
+        routes={[]}
+        renderNavigation={() => <Navigation />}
+      />
+    );
+
+    expect(wrapper.find('NavMask').prop('isVisible')).toBeFalsy();
+    expect(wrapper.find('SidebarWrapper').prop('isVisible')).toBeFalsy();
+
+    wrapper.find('NavMenuToggle').simulate('click');
+
+    expect(wrapper.find('NavMask').prop('isVisible')).toBeTruthy();
+    expect(wrapper.find('SidebarWrapper').prop('isVisible')).toBeTruthy();
+  });
 });

Some files were not shown because too many files changed in this diff