Refactored select behaviour to be better readable

- Remove header from Search & Settings on mobile

Co-authored-by: Tejas Kumar <tejas@tejas.qa>
Signed-off-by: Philipp Hugenroth <philipph@spotify.com>

packages/app/src/components/Root/Root.tsx

@@ -32,12 +32,7 @@ import {
   Settings as SidebarSettings,
   UserSettingsSignInAvatar,
 } from '@backstage/plugin-user-settings';
-import {
-  BottomNavigation,
-  BottomNavigationAction,
-  Link,
-  makeStyles,
-} from '@material-ui/core';
+import { Link, makeStyles } from '@material-ui/core';
 import CreateComponentIcon from '@material-ui/icons/AddCircleOutline';
 import RuleIcon from '@material-ui/icons/AssignmentTurnedIn';
 import ExtensionIcon from '@material-ui/icons/Extension';
@@ -48,7 +43,7 @@ import MenuIcon from '@material-ui/icons/Menu';
 import MoneyIcon from '@material-ui/icons/MonetizationOn';
 import MapIcon from '@material-ui/icons/MyLocation';
 import SearchIcon from '@material-ui/icons/Search';
-import React, { PropsWithChildren, useContext, useState } from 'react';
+import React, { PropsWithChildren, useContext } from 'react';
 import { NavLink } from 'react-router-dom';
 import LogoFull from './LogoFull';
 import LogoIcon from './LogoIcon';

packages/app/src/components/search/SearchPage.tsx

@@ -25,8 +25,18 @@ import {
   SearchType,
 } from '@backstage/plugin-search';
 import { DocsResultListItem } from '@backstage/plugin-techdocs';
-import { Grid, List, makeStyles, Paper, Theme } from '@material-ui/core';
-import React from 'react';
+import { SearchResultSet } from '@backstage/search-common';
+import { BackstageTheme } from '@backstage/theme';
+import {
+  Grid,
+  List,
+  makeStyles,
+  Paper,
+  Theme,
+  useMediaQuery,
+} from '@material-ui/core';
+import Pagination from '@material-ui/lab/Pagination';
+import React, { useState } from 'react';
 
 const useStyles = makeStyles((theme: Theme) => ({
   bar: {
@@ -44,9 +54,16 @@ const useStyles = makeStyles((theme: Theme) => ({
 
 const SearchPage = () => {
   const classes = useStyles();
+  // Think about abstraction to utils here
+  const isMobileScreen = useMediaQuery<BackstageTheme>(theme =>
+    theme.breakpoints.up('xs'),
+  );
+
   return (
     <Page themeId="home">
-      <Header title="Search" subtitle={<Lifecycle alpha />} />
+      {!isMobileScreen && (
+        <Header title="Search" subtitle={<Lifecycle alpha />} />
+      )}
       <Content>
         <Grid container direction="row">
           <Grid item xs={12}>
@@ -54,26 +71,28 @@ const SearchPage = () => {
               <SearchBar debounceTime={100} />
             </Paper>
           </Grid>
-          <Grid item xs={3}>
-            <Paper className={classes.filters}>
-              <SearchType
-                values={['techdocs', 'software-catalog']}
-                name="type"
-                defaultValue="software-catalog"
-              />
-              <SearchFilter.Select
-                className={classes.filter}
-                name="kind"
-                values={['Component', 'Template']}
-              />
-              <SearchFilter.Checkbox
-                className={classes.filter}
-                name="lifecycle"
-                values={['experimental', 'production']}
-              />
-            </Paper>
-          </Grid>
-          <Grid item xs={9}>
+          {!isMobileScreen && (
+            <Grid item xs={3}>
+              <Paper className={classes.filters}>
+                <SearchType
+                  values={['techdocs', 'software-catalog']}
+                  name="type"
+                  defaultValue="software-catalog"
+                />
+                <SearchFilter.Select
+                  className={classes.filter}
+                  name="kind"
+                  values={['Component', 'Template']}
+                />
+                <SearchFilter.Checkbox
+                  className={classes.filter}
+                  name="lifecycle"
+                  values={['experimental', 'production']}
+                />
+              </Paper>
+            </Grid>
+          )}
+          <Grid item xs>
             <SearchResult>
               {({ results }) => (
                 <List>

packages/core-components/src/layout/Sidebar/MobileSidebar.tsx

@@ -14,83 +14,128 @@
  * limitations under the License.
  */
 
-import { BottomNavigation, Box, makeStyles } from '@material-ui/core';
-import React, { useState, useEffect } from 'react';
+import { BackstageTheme } from '@backstage/theme';
+import {
+  BottomNavigation,
+  Box,
+  IconButton,
+  makeStyles,
+  Typography,
+} from '@material-ui/core';
+import React, { createContext, useEffect, useState } from 'react';
 import { useLocation } from 'react-router';
-import { SidebarGroup } from './Group';
+import { sidebarConfig } from './config';
+import { SidebarGroup } from './SidebarGroup';
+import CloseIcon from '@material-ui/icons/Close';
 
-const useStyles = makeStyles({
+type MobileSidebarContextType = {
+  selectedMenuItemIndex: number;
+  setSelectedMenuItemIndex: React.Dispatch<React.SetStateAction<number>>;
+};
+
+const useStyles = makeStyles<BackstageTheme>(theme => ({
   root: {
     position: 'fixed',
+    backgroundColor: theme.palette.navigation.background,
+    color: theme.palette.navigation.color,
     bottom: 0,
     left: 0,
     right: 0,
     zIndex: 1000,
+    borderTop: '1px solid #383838',
   },
+
+  overlay: {
+    background: theme.palette.navigation.background,
+    width: '100%',
+    flex: '0 1 auto',
+    height: `calc(100% - ${sidebarConfig.mobileSidebarHeight}px)`,
+    overflow: 'auto',
+    position: 'fixed',
+    zIndex: 500,
+  },
+
+  overlayHeader: {
+    display: 'flex',
+    color: theme.palette.bursts.fontColor,
+    alignItems: 'center',
+    justifyContent: 'space-between',
+    padding: `${theme.spacing(2)}px ${theme.spacing(3)}px`,
+  },
+
+  overlayHeaderClose: {
+    color: theme.palette.bursts.fontColor,
+  },
+}));
+
+const OverlayMenu = ({
+  children,
+  label,
+  onClose,
+}: React.PropsWithChildren<{ label: string; onClose: () => void }>) => {
+  const classes = useStyles();
+
+  return (
+    <Box className={classes.overlay}>
+      <Box className={classes.overlayHeader}>
+        <Typography variant="h3">{label}</Typography>
+        <IconButton
+          onClick={onClose}
+          classes={{ root: classes.overlayHeaderClose }}
+        >
+          <CloseIcon />
+        </IconButton>
+      </Box>
+      <Box>{children}</Box>
+    </Box>
+  );
+};
+
+export const MobileSidebarContext = createContext<MobileSidebarContextType>({
+  selectedMenuItemIndex: -1,
+  setSelectedMenuItemIndex: () => {},
 });
 
-const OverlayMenu = ({ children }: React.PropsWithChildren<{}>) => (
-  <Box
-    style={{
-      background: 'black',
-      width: '100%',
-      height: '100%',
-      position: 'fixed',
-      zIndex: 500,
-    }}
-  >
-    {children}
-  </Box>
-);
-
-let currentLocation: string;
-
 /**
  * Filters for sidebar groups and reorders them to create a custom BottomNavigation
  */
 export const MobileSidebar = ({ children }: React.PropsWithChildren<{}>) => {
   const classes = useStyles();
-  const [value, setValue] = useState<number | undefined>();
-  const [menu, setMenu] = useState<React.ReactNode | undefined>();
   const location = useLocation();
+  const [selectedMenuItemIndex, setSelectedMenuItemIndex] =
+    useState<number>(-1);
 
   useEffect(() => {
-    if (menu && currentLocation !== location.pathname) {
-      setMenu(undefined);
-      setValue(undefined);
-    }
-    currentLocation = location.pathname;
-  }, [location, menu]);
+    // This is getting triggered to often - fix me!
+    setSelectedMenuItemIndex(-1);
+  }, [location.pathname]);
+
+  const sidebarGroups = React.Children.map(children, child =>
+    React.isValidElement(child) && child.type === SidebarGroup ? child : null,
+  );
+
+  if (!sidebarGroups) {
+    // error api
+    return null; // think about the exception state
+  }
+
+  const shouldShowGroupChildren =
+    selectedMenuItemIndex >= 0 &&
+    !sidebarGroups[selectedMenuItemIndex].props.to;
 
   return (
-    <React.Fragment>
-      {value && menu && <OverlayMenu children={menu} />}
+    <MobileSidebarContext.Provider
+      value={{ selectedMenuItemIndex, setSelectedMenuItemIndex }}
+    >
+      {shouldShowGroupChildren && (
+        <OverlayMenu
+          {...sidebarGroups[selectedMenuItemIndex].props}
+          onClose={() => setSelectedMenuItemIndex(-1)}
+        />
+      )}
       <BottomNavigation className={classes.root}>
-        {React.Children.map(children, (child, index) => {
-          if (
-            React.isValidElement(child) &&
-            (child as React.ReactElement).type === SidebarGroup
-          ) {
-            if (index === value && !menu && !child.props.to) {
-              setMenu(child.props.children);
-            }
-            return React.cloneElement(child, {
-              injectedSelected: value
-                ? index === value
-                : location.pathname === child.props.to,
-              onClick: () => {
-                if (index === value && menu) {
-                  setValue(undefined);
-                  setMenu(undefined);
-                } else {
-                  setValue(index);
-                }
-              },
-            });
-          }
-          return null;
-        })}
+        {sidebarGroups}
       </BottomNavigation>
-    </React.Fragment>
+    </MobileSidebarContext.Provider>
   );
 };

packages/core-components/src/layout/Sidebar/SidebarGroup.tsx

@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/no-shadow */
 /*
  * Copyright 2020 The Backstage Authors
  *
@@ -14,24 +15,34 @@
  * limitations under the License.
  */
 
+import { BackstageTheme } from '@backstage/theme';
 import {
   BottomNavigationAction,
   BottomNavigationActionProps,
   makeStyles,
 } from '@material-ui/core';
-import React from 'react';
-import { NavLink, useLocation } from 'react-router-dom';
+import React, { useContext } from 'react';
+import { useLocation } from 'react-router-dom';
+import { Link } from '../../components';
+import { MobileSidebarContext } from './MobileSidebar';
 
 interface SidebarGroupProps extends BottomNavigationActionProps {
   to?: string;
-  injectedSelected?: boolean;
 }
 
-const useStyles = makeStyles({
+const useStyles = makeStyles<BackstageTheme>(theme => ({
+  root: {
+    color: theme.palette.navigation.color,
+  },
+
+  selected: {
+    color: `${theme.palette.navigation.selectedColor}!important`,
+  },
+
   label: {
     display: 'none',
   },
-});
+}));
 
 /**
  * If the page is mobile it should be BottomNavigationAction - otherwise just a fragment
@@ -42,21 +53,35 @@ export const SidebarGroup = ({
   to,
   label,
   icon,
-  onClick,
-  injectedSelected,
+  value,
 }: React.PropsWithChildren<SidebarGroupProps>) => {
   const classes = useStyles();
   const location = useLocation();
+  const { selectedMenuItemIndex, setSelectedMenuItemIndex } =
+    useContext(MobileSidebarContext);
+
+  const onChange = (_, value) => {
+    if (value === selectedMenuItemIndex && to !== location.pathname) {
+      setSelectedMenuItemIndex(-1);
+    } else if (value !== selectedMenuItemIndex) {
+      setSelectedMenuItemIndex(value);
+    }
+  };
+
+  const selected =
+    value === selectedMenuItemIndex ||
+    (selectedMenuItemIndex >= 0 && to === location.pathname);
 
   return (
+    // @ts-ignore Material UI issue: https://github.com/mui-org/material-ui/issues/27820
     <BottomNavigationAction
       label={label}
       icon={icon}
-      component={NavLink}
+      component={Link}
       to={to ? to : location.pathname}
-      onClick={onClick}
-      value={to}
-      selected={injectedSelected}
+      onChange={onChange}
+      value={value}
+      selected={selected}
       classes={classes}
     />
   );

packages/core-components/src/layout/Sidebar/config.ts

@@ -35,6 +35,7 @@ export const sidebarConfig = {
   selectedIndicatorWidth: 3,
   userBadgePadding,
   userBadgeDiameter: drawerWidthClosed - userBadgePadding * 2,
+  mobileSidebarHeight: 56,
 };
 
 export const SIDEBAR_INTRO_LOCAL_STORAGE =

packages/core-components/src/layout/Sidebar/index.ts

@@ -16,7 +16,7 @@
 
 export { Sidebar } from './Bar';
 export { MobileSidebar } from './MobileSidebar';
-export { SidebarGroup } from './Group';
+export { SidebarGroup } from './SidebarGroup';
 export { SidebarPage, SidebarPinStateContext } from './Page';
 export type { SidebarPinStateContextType, SidebarPageClassKey } from './Page';
 export {

plugins/user-settings/src/components/General/UserSettingsGeneral.tsx

@@ -21,10 +21,10 @@ import { UserSettingsAppearanceCard } from './UserSettingsAppearanceCard';
 export const UserSettingsGeneral = () => {
   return (
     <Grid container direction="row" spacing={3}>
-      <Grid item sm={12} md={6}>
+      <Grid item xs={12} md={6}>
         <UserSettingsProfileCard />
       </Grid>
-      <Grid item sm={12} md={6}>
+      <Grid item xs={12} md={6}>
         <UserSettingsAppearanceCard />
       </Grid>
     </Grid>

plugins/user-settings/src/components/SettingsPage.tsx

@@ -14,21 +14,26 @@
  * limitations under the License.
  */
 
+import { Header, Page, TabbedLayout } from '@backstage/core-components';
+import { BackstageTheme } from '@backstage/theme';
+import { useMediaQuery } from '@material-ui/core';
 import React from 'react';
 import { UserSettingsAuthProviders } from './AuthProviders';
 import { UserSettingsFeatureFlags } from './FeatureFlags';
 import { UserSettingsGeneral } from './General';
-import { Header, Page, TabbedLayout } from '@backstage/core-components';
 
 type Props = {
   providerSettings?: JSX.Element;
 };
 
 export const SettingsPage = ({ providerSettings }: Props) => {
+  const isMobileScreen = useMediaQuery<BackstageTheme>(theme =>
+    theme.breakpoints.up('xs'),
+  );
+
   return (
     <Page themeId="home">
-      <Header title="Settings" />
-
+      {!isMobileScreen && <Header title="Settings" />}
       <TabbedLayout>
         <TabbedLayout.Route path="general" title="General">
           <UserSettingsGeneral />