From 42988939608ea298622f06f0b0cfad7bfd5c0bd4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Sep 2023 11:33:57 +0200 Subject: [PATCH 1/6] Improve useEffect usage in App component, to have the right dependencies --- src/app/App.tsx | 12 ++++++++---- src/settings/UserInterfaceSettings.tsx | 4 +--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/app/App.tsx b/src/app/App.tsx index b27811a7..bb9d4022 100644 --- a/src/app/App.tsx +++ b/src/app/App.tsx @@ -1,7 +1,7 @@ import { changeThemeInMarkup } from '@shlinkio/shlink-frontend-kit'; import classNames from 'classnames'; import type { FC } from 'react'; -import { useEffect } from 'react'; +import { useEffect, useRef } from 'react'; import { Route, Routes, useLocation } from 'react-router-dom'; import { AppUpdateBanner } from '../common/AppUpdateBanner'; import { NotFound } from '../common/NotFound'; @@ -29,16 +29,20 @@ export const App = ( ShlinkVersionsContainer: FC, ) => ({ fetchServers, servers, settings, appUpdated, resetAppUpdate }: AppProps) => { const location = useLocation(); + const initialServers = useRef(servers); const isHome = location.pathname === '/'; useEffect(() => { - // On first load, try to fetch the remote servers if the list is empty - if (Object.keys(servers).length === 0) { + // Try to fetch the remote servers if the list is empty at first + // We use a ref because we don't care if the servers list becomes empty later + if (Object.keys(initialServers.current).length === 0) { fetchServers(); } + }, [fetchServers]); + useEffect(() => { changeThemeInMarkup(settings.ui?.theme ?? 'light'); - }, []); + }, [settings.ui?.theme]); return (
diff --git a/src/settings/UserInterfaceSettings.tsx b/src/settings/UserInterfaceSettings.tsx index 58f2a086..01d8d29e 100644 --- a/src/settings/UserInterfaceSettings.tsx +++ b/src/settings/UserInterfaceSettings.tsx @@ -1,7 +1,7 @@ import { faMoon, faSun } from '@fortawesome/free-solid-svg-icons'; import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; import type { Theme } from '@shlinkio/shlink-frontend-kit'; -import { changeThemeInMarkup, SimpleCard, ToggleSwitch } from '@shlinkio/shlink-frontend-kit'; +import { SimpleCard, ToggleSwitch } from '@shlinkio/shlink-frontend-kit'; import type { FC } from 'react'; import type { AppSettings, UiSettings } from './reducers/settings'; import './UserInterfaceSettings.scss'; @@ -18,9 +18,7 @@ export const UserInterfaceSettings: FC = ({ settings: { ui } checked={ui?.theme === 'dark'} onChange={(useDarkTheme) => { const theme: Theme = useDarkTheme ? 'dark' : 'light'; - setUiSettings({ ...ui, theme }); - changeThemeInMarkup(theme); }} > Use dark theme. From fbc47846e3e1a09116bd2ace2edc9c071064377f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 2 Sep 2023 19:08:12 +0200 Subject: [PATCH 2/6] Fix some instances of react-hooks/exhaustive-deps --- package-lock.json | 14 +++++++------- package.json | 2 +- src/common/Home.tsx | 2 +- src/common/MainHeader.tsx | 11 ++++++----- src/servers/CreateServer.tsx | 26 +++++++++++++------------- test/servers/CreateServer.test.tsx | 24 +++++++++++++++++++----- 6 files changed, 47 insertions(+), 32 deletions(-) diff --git a/package-lock.json b/package-lock.json index 3a2adf85..40082368 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,7 +15,7 @@ "@fortawesome/react-fontawesome": "^0.2.0", "@json2csv/plainjs": "^7.0.3", "@reduxjs/toolkit": "^1.9.5", - "@shlinkio/shlink-frontend-kit": "^0.2.0", + "@shlinkio/shlink-frontend-kit": "^0.2.1", "@shlinkio/shlink-js-sdk": "^0.1.0", "@shlinkio/shlink-web-component": "^0.3.3", "bootstrap": "5.2.3", @@ -2801,9 +2801,9 @@ } }, "node_modules/@shlinkio/shlink-frontend-kit": { - "version": "0.2.0", - "resolved": "https://registry.npmjs.org/@shlinkio/shlink-frontend-kit/-/shlink-frontend-kit-0.2.0.tgz", - "integrity": "sha512-8kGaae0bTiGzbLzPsolLvJ5ud37BR2b1WeDy8lyXIiwoFiSAMIgWpqro0nMdBVBQXovjmMbtiS6BFYsaoBo9/g==", + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/@shlinkio/shlink-frontend-kit/-/shlink-frontend-kit-0.2.1.tgz", + "integrity": "sha512-5mRpQII9bGHAJQ1ghgGY+jFC5tD0y0ufgPqco9vLVGXGVf+BSNQrTiw/Cx6f9eCInqFPgFo8CdJzWUHbIzvC+Q==", "dependencies": { "classnames": "^2.3.2", "qs": "^6.11.2", @@ -12390,9 +12390,9 @@ } }, "@shlinkio/shlink-frontend-kit": { - "version": "0.2.0", - "resolved": "https://registry.npmjs.org/@shlinkio/shlink-frontend-kit/-/shlink-frontend-kit-0.2.0.tgz", - "integrity": "sha512-8kGaae0bTiGzbLzPsolLvJ5ud37BR2b1WeDy8lyXIiwoFiSAMIgWpqro0nMdBVBQXovjmMbtiS6BFYsaoBo9/g==", + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/@shlinkio/shlink-frontend-kit/-/shlink-frontend-kit-0.2.1.tgz", + "integrity": "sha512-5mRpQII9bGHAJQ1ghgGY+jFC5tD0y0ufgPqco9vLVGXGVf+BSNQrTiw/Cx6f9eCInqFPgFo8CdJzWUHbIzvC+Q==", "requires": { "classnames": "^2.3.2", "qs": "^6.11.2", diff --git a/package.json b/package.json index 0c79b7a6..a064e41e 100644 --- a/package.json +++ b/package.json @@ -31,7 +31,7 @@ "@fortawesome/react-fontawesome": "^0.2.0", "@json2csv/plainjs": "^7.0.3", "@reduxjs/toolkit": "^1.9.5", - "@shlinkio/shlink-frontend-kit": "^0.2.0", + "@shlinkio/shlink-frontend-kit": "^0.2.1", "@shlinkio/shlink-js-sdk": "^0.1.0", "@shlinkio/shlink-web-component": "^0.3.3", "bootstrap": "5.2.3", diff --git a/src/common/Home.tsx b/src/common/Home.tsx index 4029e39b..bf7b79bd 100644 --- a/src/common/Home.tsx +++ b/src/common/Home.tsx @@ -23,7 +23,7 @@ export const Home = ({ servers }: HomeProps) => { // Try to redirect to the first server marked as auto-connect const autoConnectServer = serversList.find(({ autoConnect }) => autoConnect); autoConnectServer && navigate(`/server/${autoConnectServer.id}`); - }, []); + }, [serversList, navigate]); return (
diff --git a/src/common/MainHeader.tsx b/src/common/MainHeader.tsx index 74cd028a..750ba744 100644 --- a/src/common/MainHeader.tsx +++ b/src/common/MainHeader.tsx @@ -10,14 +10,15 @@ import { ShlinkLogo } from './img/ShlinkLogo'; import './MainHeader.scss'; export const MainHeader = (ServersDropdown: FC) => () => { - const [isOpen, toggleOpen, , close] = useToggle(); + const [isNotCollapsed, toggleCollapse, , collapse] = useToggle(); const location = useLocation(); const { pathname } = location; - useEffect(close, [location]); + // In mobile devices, collapse the navbar when location changes + useEffect(collapse, [location, collapse]); const settingsPath = '/settings'; - const toggleClass = classNames('main-header__toggle-icon', { 'main-header__toggle-icon--opened': isOpen }); + const toggleClass = classNames('main-header__toggle-icon', { 'main-header__toggle-icon--opened': isNotCollapsed }); return ( @@ -25,11 +26,11 @@ export const MainHeader = (ServersDropdown: FC) => () => { Shlink - + - +