Merge pull request #577 from acelaya-forks/feature/enhanced-settings

Feature/enhanced settings

CHANGELOG.md

@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 * [#570](https://github.com/shlinkio/shlink-web-client/pull/570) Added new section to load non-orphan visits all together.
 
 ### Changed
+* [#543](https://github.com/shlinkio/shlink-web-client/pull/543) Redesigned settings section.
 * [#567](https://github.com/shlinkio/shlink-web-client/pull/567) Improved Shlink 3.0.0 compatibility by checking the `INVALID_SHORT_URL_DELETION` error code when deleting short URLs.
 * [#524](https://github.com/shlinkio/shlink-web-client/pull/524) Updated to react-router v6.
 

src/app/App.tsx

@@ -47,7 +47,7 @@ const App = (
         <div className={classNames('shlink-wrapper', { 'd-flex d-md-block align-items-center': isHome })}>
           <Routes>
             <Route index element={<Home />} />
-            <Route path="/settings" element={<Settings />} />
+            <Route path="/settings/*" element={<Settings />} />
             <Route path="/manage-servers" element={<ManageServers />} />
             <Route path="/server/create" element={<CreateServer />} />
             <Route path="/server/:serverId/edit" element={<EditServer />} />

src/common/MainHeader.tsx

@@ -31,7 +31,7 @@ const MainHeader = (ServersDropdown: FC) => () => {
       <Collapse navbar isOpen={isOpen}>
         <Nav navbar className="ml-auto">
           <NavItem>
-            <NavLink tag={Link} to={settingsPath} active={pathname === settingsPath}>
+            <NavLink tag={Link} to={settingsPath} active={pathname.startsWith(settingsPath)}>
               <FontAwesomeIcon icon={cogsIcon} />&nbsp; Settings
             </NavLink>
           </NavItem>

src/settings/Settings.tsx

@@ -1,18 +1,11 @@
 import { FC, ReactNode } from 'react';
-import { Row } from 'reactstrap';
+import { Navigate, Routes, Route } from 'react-router-dom';
 import { NoMenuLayout } from '../common/NoMenuLayout';
+import { NavPillItem, NavPills } from '../utils/NavPills';
 
-const SettingsSections: FC<{ items: ReactNode[][] }> = ({ items }) => (
+const SettingsSections: FC<{ items: ReactNode[] }> = ({ items }) => (
   <>
-    {items.map((child, index) => (
-      <Row key={index}>
-        {child.map((subChild, subIndex) => (
-          <div key={subIndex} className={`col-lg-${12 / child.length} mb-3`}>
-            {subChild}
-          </div>
-        ))}
-      </Row>
-    ))}
+    {items.map((child, index) => <div key={index} className="mb-3">{child}</div>)}
   </>
 );
 
@@ -25,13 +18,18 @@ const Settings = (
   Tags: FC,
 ) => () => (
   <NoMenuLayout>
-    <SettingsSections
-      items={[
-        [ <UserInterface />, <Visits /> ], // eslint-disable-line react/jsx-key
-        [ <ShortUrlCreation />, <ShortUrlsList /> ], // eslint-disable-line react/jsx-key
-        [ <Tags />, <RealTimeUpdates /> ], // eslint-disable-line react/jsx-key
-      ]}
-    />
+    <NavPills>
+      <NavPillItem to="general">General</NavPillItem>
+      <NavPillItem to="short-urls">Short URLs</NavPillItem>
+      <NavPillItem to="secondary-items">Secondary items</NavPillItem>
+    </NavPills>
+
+    <Routes>
+      <Route path="general" element={<SettingsSections items={[ <UserInterface key="one" />, <RealTimeUpdates key="two" /> ]} />} />
+      <Route path="short-urls" element={<SettingsSections items={[ <ShortUrlCreation key="one" />, <ShortUrlsList key="two" /> ]} />} />
+      <Route path="secondary-items" element={<SettingsSections items={[ <Tags key="one" />, <Visits key="two" /> ]} />} />
+      <Route path="*" element={<Navigate replace to="general" />} />
+    </Routes>
   </NoMenuLayout>
 );
 

src/settings/UserInterfaceSettings.tsx

@@ -1,7 +1,6 @@
 import { FC } from 'react';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { faSun, faMoon } from '@fortawesome/free-solid-svg-icons';
-import { FormGroup } from 'reactstrap';
 import { SimpleCard } from '../utils/SimpleCard';
 import ToggleSwitch from '../utils/ToggleSwitch';
 import { changeThemeInMarkup, Theme } from '../utils/theme';
@@ -15,7 +14,6 @@ interface UserInterfaceProps {
 
 export const UserInterfaceSettings: FC<UserInterfaceProps> = ({ settings: { ui }, setUiSettings }) => (
   <SimpleCard title="User interface" className="h-100">
-    <FormGroup>
     <FontAwesomeIcon icon={ui?.theme === 'dark' ? faMoon : faSun} className="user-interface__theme-icon" />
     <ToggleSwitch
       checked={ui?.theme === 'dark'}
@@ -28,6 +26,5 @@ export const UserInterfaceSettings: FC<UserInterfaceProps> = ({ settings: { ui }
     >
       Use dark theme.
     </ToggleSwitch>
-    </FormGroup>
   </SimpleCard>
 );

src/utils/NavPills.scss

@@ -1,12 +1,12 @@
-@import '../utils/base';
+@import './base';
 
-.visits-stats__nav {
+.nav-pills__nav {
   position: sticky !important;
   top: $headerHeight - 1px;
   z-index: 2;
 }
 
-.visits-stats__nav-link {
+.nav-pills__nav-link {
   border-radius: 0 !important;
   padding-bottom: calc(.5rem - 3px) !important;
   border-bottom: 3px solid transparent;
@@ -19,11 +19,11 @@
   }
 }
 
-.visits-stats__nav-link:hover {
+.nav-pills__nav-link:hover {
   color: $mainColor !important;
 }
 
-.visits-stats__nav-link.active {
+.nav-pills__nav-link.active {
   border-color: $mainColor;
   background-color: var(--primary-color) !important;
   color: $mainColor !important;

src/utils/NavPills.tsx

@@ -0,0 +1,29 @@
+import { FC, Children, isValidElement } from 'react';
+import { Card, Nav, NavLink } from 'reactstrap';
+import { NavLink as RouterNavLink } from 'react-router-dom';
+import './NavPills.scss';
+
+interface NavPillProps {
+  to: string;
+  replace?: boolean;
+}
+
+export const NavPillItem: FC<NavPillProps> = ({ children, ...rest }) => (
+  <NavLink className="nav-pills__nav-link" tag={RouterNavLink} {...rest}>
+    {children}
+  </NavLink>
+);
+
+export const NavPills: FC<{ fill?: boolean }> = ({ children, fill = false }) => (
+  <Card className="nav-pills__nav p-0 overflow-hidden mb-3" body>
+    <Nav pills fill={fill}>
+      {Children.map(children, (child) => {
+        if (!isValidElement(child) || child.type !== NavPillItem) {
+          throw new Error('Only NavPillItem children are allowed inside NavPills.');
+        }
+
+        return child;
+      })}
+    </Nav>
+  </Card>
+);

src/visits/VisitsStats.tsx

@@ -1,11 +1,10 @@
 import { isEmpty, propEq, values } from 'ramda';
 import { useState, useEffect, useMemo, FC, useRef } from 'react';
-import { Button, Card, Nav, NavLink, Progress, Row } from 'reactstrap';
+import { Button, Progress, Row } from 'reactstrap';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import { faCalendarAlt, faMapMarkedAlt, faList, faChartPie, faFileDownload } from '@fortawesome/free-solid-svg-icons';
 import { IconDefinition } from '@fortawesome/fontawesome-common-types';
-import { Route, Routes, NavLink as RouterNavLink, Navigate } from 'react-router-dom';
-import { Location } from 'history';
+import { Route, Routes, Navigate } from 'react-router-dom';
 import classNames from 'classnames';
 import { DateRangeSelector } from '../utils/dates/DateRangeSelector';
 import Message from '../utils/Message';
@@ -16,6 +15,7 @@ import { Settings } from '../settings/reducers/settings';
 import { SelectedServer } from '../servers/data';
 import { supportsBotVisits } from '../utils/helpers/features';
 import { prettify } from '../utils/helpers/numbers';
+import { NavPillItem, NavPills } from '../utils/NavPills';
 import LineChartCard from './charts/LineChartCard';
 import VisitsTable from './VisitsTable';
 import { NormalizedOrphanVisit, NormalizedVisit, VisitsFilter, VisitsInfo, VisitsParams } from './types';
@@ -25,7 +25,6 @@ import { VisitsFilterDropdown } from './helpers/VisitsFilterDropdown';
 import { HighlightableProps, highlightedVisitsToStats } from './types/helpers';
 import { DoughnutChartCard } from './charts/DoughnutChartCard';
 import { SortableBarChartCard } from './charts/SortableBarChartCard';
-import './VisitsStats.scss';
 
 export interface VisitsStatsProps {
   getVisits: (params: VisitsParams, doIntervalFallback?: boolean) => void;
@@ -55,19 +54,6 @@ const sections: Record<Section, VisitsNavLinkProps> = {
 
 let selectedBar: string | undefined;
 
-const VisitsNavLink: FC<VisitsNavLinkProps & { to: string }> = ({ subPath, title, icon, to }) => (
-  <NavLink
-    tag={RouterNavLink}
-    className="visits-stats__nav-link"
-    to={to}
-    isActive={(_: null, { pathname }: Location) => pathname.endsWith(`visits${subPath}`)}
-    replace
-  >
-    <FontAwesomeIcon icon={icon} />
-    <span className="ml-2 d-none d-sm-inline">{title}</span>
-  </NavLink>
-);
-
 const VisitsStats: FC<VisitsStatsProps> = ({
   children,
   visitsInfo,
@@ -157,12 +143,14 @@ const VisitsStats: FC<VisitsStatsProps> = ({
 
     return (
       <>
-        <Card className="visits-stats__nav p-0 overflow-hidden" body>
-          <Nav pills fill>
-            {Object.entries(sections).map(([ section, props ]) =>
-              <VisitsNavLink key={section} {...props} to={buildSectionUrl(props.subPath)} />)}
-          </Nav>
-        </Card>
+        <NavPills fill>
+          {Object.values(sections).map(({ title, icon, subPath }, index) => (
+            <NavPillItem key={index} to={buildSectionUrl(subPath)} replace>
+              <FontAwesomeIcon icon={icon} />
+              <span className="ml-2 d-none d-sm-inline">{title}</span>
+            </NavPillItem>
+          ))}
+        </NavPills>
         <Row>
           <Routes>
             <Route

test/app/App.test.tsx

@@ -54,7 +54,7 @@ describe('<App />', () => {
     const routes = wrapper.find(Route);
     const expectedPaths = [
       undefined,
-      '/settings',
+      '/settings/*',
       '/manage-servers',
       '/server/create',
       '/server/:serverId/edit',

test/common/MainHeader.test.tsx

@@ -35,6 +35,8 @@ describe('<MainHeader />', () => {
     [ '/foo', false ],
     [ '/bar', false ],
     [ '/settings', true ],
+    [ '/settings/foo', true ],
+    [ '/settings/bar', true ],
   ])('sets link to settings as active only when current path is settings', (currentPath, isActive) => {
     const wrapper = createWrapper(currentPath);
     const settingsLink = wrapper.find(NavLink);

test/settings/Settings.test.tsx

@@ -1,6 +1,8 @@
 import { shallow } from 'enzyme';
+import { Route } from 'react-router-dom';
 import createSettings from '../../src/settings/Settings';
 import { NoMenuLayout } from '../../src/common/NoMenuLayout';
+import { NavPillItem } from '../../src/utils/NavPills';
 
 describe('<Settings />', () => {
   const Component = () => null;
@@ -9,10 +11,19 @@ describe('<Settings />', () => {
   it('renders a no-menu layout with the expected settings sections', () => {
     const wrapper = shallow(<Settings />);
     const layout = wrapper.find(NoMenuLayout);
-    const sections = wrapper.find('SettingsSections');
+    const sections = wrapper.find(Route);
 
     expect(layout).toHaveLength(1);
-    expect(sections).toHaveLength(1);
-    expect((sections.prop('items') as any[]).flat()).toHaveLength(6);
+    expect(sections).toHaveLength(4);
+  });
+
+  it('renders expected menu', () => {
+    const wrapper = shallow(<Settings />);
+    const items = wrapper.find(NavPillItem);
+
+    expect(items).toHaveLength(3);
+    expect(items.first().prop('to')).toEqual('general');
+    expect(items.at(1).prop('to')).toEqual('short-urls');
+    expect(items.last().prop('to')).toEqual('secondary-items');
   });
 });

test/utils/NavPills.test.tsx

@@ -0,0 +1,41 @@
+import { shallow } from 'enzyme';
+import { Card, Nav } from 'reactstrap';
+import { NavPillItem, NavPills } from '../../src/utils/NavPills';
+
+describe('<NavPills />', () => {
+  it.each([
+    [ 'Foo' ],
+    [ <span key="1">Hi!</span> ],
+    [[ <NavPillItem key="1" to="" />, <span key="2">Hi!</span> ]],
+  ])('throws error when any of the children is not a NavPillItem', (children) => {
+    expect.assertions(1);
+
+    try {
+      shallow(<NavPills>{children}</NavPills>);
+    } catch (e: any) {
+      expect(e.message).toEqual('Only NavPillItem children are allowed inside NavPills.');
+    }
+  });
+
+  it.each([
+    [ undefined ],
+    [ true ],
+    [ false ],
+  ])('renders provided items', (fill) => {
+    const wrapper = shallow(
+      <NavPills fill={fill}>
+        <NavPillItem to="1">1</NavPillItem>
+        <NavPillItem to="2">2</NavPillItem>
+        <NavPillItem to="3">3</NavPillItem>
+      </NavPills>,
+    );
+    const card = wrapper.find(Card);
+    const nav = wrapper.find(Nav);
+
+    expect(card).toHaveLength(1);
+    expect(card.prop('body')).toEqual(true);
+    expect(nav).toHaveLength(1);
+    expect(nav.prop('pills')).toEqual(true);
+    expect(nav.prop('fill')).toEqual(!!fill);
+  });
+});