From 5a9640bd57fe02d022c9f85b46a8ee35a1f37a48 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 6 Aug 2023 18:07:03 +0200 Subject: [PATCH] Remove sidebar reducer, which couple web-client with web-component --- shlink-web-component/src/Main.tsx | 13 ++++++---- src/common/ShlinkVersionsContainer.tsx | 17 +++++++----- src/common/ShlinkWebComponentContainer.tsx | 12 +-------- src/common/reducers/sidebar.ts | 26 ------------------- src/common/services/provideServices.ts | 12 ++------- src/container/types.ts | 2 -- src/reducers/index.ts | 2 -- test/common/ShlinkVersionsContainer.test.tsx | 21 ++++++++++----- .../ShlinkWebComponentContainer.test.tsx | 8 +----- test/common/reducer/sidebar.test.ts | 12 --------- 10 files changed, 38 insertions(+), 87 deletions(-) delete mode 100644 src/common/reducers/sidebar.ts delete mode 100644 test/common/reducer/sidebar.test.ts diff --git a/shlink-web-component/src/Main.tsx b/shlink-web-component/src/Main.tsx index ab92b801..790753df 100644 --- a/shlink-web-component/src/Main.tsx +++ b/shlink-web-component/src/Main.tsx @@ -3,8 +3,8 @@ import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import { useToggle } from '@shlinkio/shlink-frontend-kit'; import classNames from 'classnames'; import type { FC, ReactNode } from 'react'; -import { useEffect } from 'react'; -import { Navigate, Route, Routes, useLocation } from 'react-router-dom'; +import { Fragment, useEffect, useMemo } from 'react'; +import { BrowserRouter, Navigate, Route, Routes, useInRouterContext, useLocation } from 'react-router-dom'; import { AsideMenu } from './common/AsideMenu'; import { useFeature } from './utils/features'; import { useSwipeable } from './utils/helpers/hooks'; @@ -30,6 +30,9 @@ export const Main = ( ): FC => ({ createNotFound }) => { const location = useLocation(); const routesPrefix = useRoutesPrefix(); + const inRouterContext = useInRouterContext(); + const Wrapper = useMemo(() => (inRouterContext ? Fragment : BrowserRouter), [inRouterContext]); + const [sidebarVisible, toggleSidebar, showSidebar, hideSidebar] = useToggle(); useEffect(() => hideSidebar(), [location]); @@ -37,10 +40,10 @@ export const Main = ( const burgerClasses = classNames('menu-layout__burger-icon', { 'menu-layout__burger-icon--active': sidebarVisible }); const swipeableProps = useSwipeable(showSidebar, hideSidebar); - // FIXME Check if this is already wrapped by a router, and wrap otherwise + // FIXME Check if this works when not currently wrapped in a router return ( - <> +
@@ -67,6 +70,6 @@ export const Main = (
- + ); }; diff --git a/src/common/ShlinkVersionsContainer.tsx b/src/common/ShlinkVersionsContainer.tsx index cdce6c3a..fcfe4567 100644 --- a/src/common/ShlinkVersionsContainer.tsx +++ b/src/common/ShlinkVersionsContainer.tsx @@ -1,17 +1,22 @@ import classNames from 'classnames'; +import { useMemo } from 'react'; +import { useLocation } from 'react-router-dom'; import type { SelectedServer } from '../servers/data'; -import type { Sidebar } from './reducers/sidebar'; import { ShlinkVersions } from './ShlinkVersions'; import './ShlinkVersionsContainer.scss'; -export interface ShlinkVersionsContainerProps { +export type ShlinkVersionsContainerProps = { selectedServer: SelectedServer; - sidebar: Sidebar; -} +}; + +const SHLINK_CONTAINER_PATH_PATTERN = /^\/server\/[a-zA-Z0-9-]*\/(?!edit)/; + +export const ShlinkVersionsContainer = ({ selectedServer }: ShlinkVersionsContainerProps) => { + const { pathname } = useLocation(); + const withPadding = useMemo(() => SHLINK_CONTAINER_PATH_PATTERN.test(pathname), [pathname]); -export const ShlinkVersionsContainer = ({ selectedServer, sidebar }: ShlinkVersionsContainerProps) => { const classes = classNames('text-center', { - 'shlink-versions-container--with-sidebar': sidebar.sidebarPresent, + 'shlink-versions-container--with-sidebar': withPadding, }); return ( diff --git a/src/common/ShlinkWebComponentContainer.tsx b/src/common/ShlinkWebComponentContainer.tsx index 7c444b5c..b1a653de 100644 --- a/src/common/ShlinkWebComponentContainer.tsx +++ b/src/common/ShlinkWebComponentContainer.tsx @@ -1,6 +1,5 @@ import type { Settings, ShlinkWebComponentType, TagColorsStorage } from '@shlinkio/shlink-web-component'; import type { FC } from 'react'; -import { useEffect } from 'react'; import type { ShlinkApiClientBuilder } from '../api/services/ShlinkApiClientBuilder'; import { isReachableServer } from '../servers/data'; import { withSelectedServer } from '../servers/helpers/withSelectedServer'; @@ -8,8 +7,6 @@ import { NotFound } from './NotFound'; import './ShlinkWebComponentContainer.scss'; interface ShlinkWebComponentContainerProps { - sidebarPresent: Function; - sidebarNotPresent: Function; settings: Settings; } @@ -18,17 +15,10 @@ export const ShlinkWebComponentContainer = ( tagColorsStorage: TagColorsStorage, ShlinkWebComponent: ShlinkWebComponentType, ServerError: FC, -) => withSelectedServer(( - { selectedServer, sidebarNotPresent, sidebarPresent, settings }, -) => { +) => withSelectedServer(({ selectedServer, settings }) => { const selectedServerIsReachable = isReachableServer(selectedServer); const routesPrefix = selectedServerIsReachable ? `/server/${selectedServer.id}` : ''; - useEffect(() => { - selectedServerIsReachable && sidebarPresent(); - return () => sidebarNotPresent(); - }, []); - if (!selectedServerIsReachable) { return ; } diff --git a/src/common/reducers/sidebar.ts b/src/common/reducers/sidebar.ts deleted file mode 100644 index eedee0cb..00000000 --- a/src/common/reducers/sidebar.ts +++ /dev/null @@ -1,26 +0,0 @@ -import { createSlice } from '@reduxjs/toolkit'; - -// FIXME This is only used for some components to have extra paddings/styles if existing section has a side menu -// Now that's basically the route which renders ShlinkWebComponent, so maybe there's some way to re-think this -// logic, and perhaps get rid of a reducer just for that - -export interface Sidebar { - sidebarPresent: boolean; -} - -const initialState: Sidebar = { - sidebarPresent: false, -}; - -const { actions, reducer } = createSlice({ - name: 'shlink/sidebar', - initialState, - reducers: { - sidebarPresent: () => ({ sidebarPresent: true }), - sidebarNotPresent: () => ({ sidebarPresent: false }), - }, -}); - -export const { sidebarPresent, sidebarNotPresent } = actions; - -export const sidebarReducer = reducer; diff --git a/src/common/services/provideServices.ts b/src/common/services/provideServices.ts index a1c38e2b..7bcb9f38 100644 --- a/src/common/services/provideServices.ts +++ b/src/common/services/provideServices.ts @@ -5,7 +5,6 @@ import { withoutSelectedServer } from '../../servers/helpers/withoutSelectedServ import { ErrorHandler } from '../ErrorHandler'; import { Home } from '../Home'; import { MainHeader } from '../MainHeader'; -import { sidebarNotPresent, sidebarPresent } from '../reducers/sidebar'; import { ScrollToTop } from '../ScrollToTop'; import { ShlinkVersionsContainer } from '../ShlinkVersionsContainer'; import { ShlinkWebComponentContainer } from '../ShlinkWebComponentContainer'; @@ -36,17 +35,10 @@ export const provideServices = (bottle: Bottle, connect: ConnectDecorator) => { 'ShlinkWebComponent', 'ServerError', ); - bottle.decorator('ShlinkWebComponentContainer', connect( - ['selectedServer', 'settings'], - ['selectServer', 'sidebarPresent', 'sidebarNotPresent'], - )); + bottle.decorator('ShlinkWebComponentContainer', connect(['selectedServer', 'settings'], ['selectServer'])); bottle.serviceFactory('ShlinkVersionsContainer', () => ShlinkVersionsContainer); - bottle.decorator('ShlinkVersionsContainer', connect(['selectedServer', 'sidebar'])); + bottle.decorator('ShlinkVersionsContainer', connect(['selectedServer'])); bottle.serviceFactory('ErrorHandler', ErrorHandler, 'window', 'console'); - - // Actions - bottle.serviceFactory('sidebarPresent', () => sidebarPresent); - bottle.serviceFactory('sidebarNotPresent', () => sidebarNotPresent); }; diff --git a/src/container/types.ts b/src/container/types.ts index 1a908f37..12aaef6a 100644 --- a/src/container/types.ts +++ b/src/container/types.ts @@ -1,5 +1,4 @@ import type { Settings } from '@shlinkio/shlink-web-component'; -import type { Sidebar } from '../common/reducers/sidebar'; import type { SelectedServer, ServersMap } from '../servers/data'; export interface ShlinkState { @@ -7,7 +6,6 @@ export interface ShlinkState { selectedServer: SelectedServer; settings: Settings; appUpdated: boolean; - sidebar: Sidebar; } export type ConnectDecorator = (props: string[] | null, actions?: string[]) => any; diff --git a/src/reducers/index.ts b/src/reducers/index.ts index 7e3a7c07..ce28c06d 100644 --- a/src/reducers/index.ts +++ b/src/reducers/index.ts @@ -1,7 +1,6 @@ import { combineReducers } from '@reduxjs/toolkit'; import type { IContainer } from 'bottlejs'; import { appUpdatesReducer } from '../app/reducers/appUpdates'; -import { sidebarReducer } from '../common/reducers/sidebar'; import type { ShlinkState } from '../container/types'; import { serversReducer } from '../servers/reducers/servers'; import { settingsReducer } from '../settings/reducers/settings'; @@ -11,5 +10,4 @@ export const initReducers = (container: IContainer) => combineReducers', () => { - const setUp = (sidebar: Sidebar) => render( - , - ); + const setUp = (activeRoute: string) => { + const history = createMemoryHistory(); + history.push(activeRoute); + + return render( + + + , + ); + }; it.each([ - [{ sidebarPresent: false }, 'text-center'], - [{ sidebarPresent: true }, 'text-center shlink-versions-container--with-sidebar'], + ['/something', 'text-center'], + ['/server/foo/edit', 'text-center'], + ['/server/foo/bar', 'text-center shlink-versions-container--with-sidebar'], ])('renders proper col classes based on sidebar status', (sidebar, expectedClasses) => { const { container } = setUp(sidebar); expect(container.firstChild).toHaveAttribute('class', `${expectedClasses}`); diff --git a/test/common/ShlinkWebComponentContainer.test.tsx b/test/common/ShlinkWebComponentContainer.test.tsx index 4d804932..54bf6411 100644 --- a/test/common/ShlinkWebComponentContainer.test.tsx +++ b/test/common/ShlinkWebComponentContainer.test.tsx @@ -17,13 +17,7 @@ describe('', () => { () => <>ServerError, ); const setUp = (selectedServer: SelectedServer) => render( - , + , ); beforeEach(() => { diff --git a/test/common/reducer/sidebar.test.ts b/test/common/reducer/sidebar.test.ts deleted file mode 100644 index 96194952..00000000 --- a/test/common/reducer/sidebar.test.ts +++ /dev/null @@ -1,12 +0,0 @@ -import { sidebarNotPresent, sidebarPresent, sidebarReducer } from '../../../src/common/reducers/sidebar'; - -describe('sidebarReducer', () => { - describe('reducer', () => { - it.each([ - [sidebarPresent, { sidebarPresent: true }], - [sidebarNotPresent, { sidebarPresent: false }], - ])('returns expected on %s', (actionCreator, expected) => { - expect(sidebarReducer(undefined, actionCreator())).toEqual(expected); - }); - }); -});