From 889d89b6ec2fece63ecb07b04fd1a54bbfb73653 Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Wed, 5 May 2021 11:34:27 +0200 Subject: [PATCH 1/5] Fix setting the state with `useQueryParamState` makes inputs lose their focus Signed-off-by: Oliver Sand --- .changeset/short-bobcats-mate.md | 6 ++++++ packages/core/src/hooks/useQueryParamState.ts | 14 +++++++++----- .../ApiExplorerTable/ApiExplorerTable.tsx | 1 - 3 files changed, 15 insertions(+), 6 deletions(-) create mode 100644 .changeset/short-bobcats-mate.md diff --git a/.changeset/short-bobcats-mate.md b/.changeset/short-bobcats-mate.md new file mode 100644 index 0000000000..22aaf11d77 --- /dev/null +++ b/.changeset/short-bobcats-mate.md @@ -0,0 +1,6 @@ +--- +'@backstage/core': patch +'@backstage/plugin-api-docs': patch +--- + +Fix setting the state with `useQueryParamState` makes inputs lose their focus. diff --git a/packages/core/src/hooks/useQueryParamState.ts b/packages/core/src/hooks/useQueryParamState.ts index da303234f5..0a61771eba 100644 --- a/packages/core/src/hooks/useQueryParamState.ts +++ b/packages/core/src/hooks/useQueryParamState.ts @@ -17,7 +17,7 @@ import { isEqual } from 'lodash'; import qs from 'qs'; import { useEffect, useState } from 'react'; -import { useLocation, useNavigate } from 'react-router-dom'; +import { useLocation } from 'react-router-dom'; import { useDebounce } from 'react-use'; function stringify(queryParams: any): string { @@ -58,9 +58,7 @@ type SetQueryParams = (params: T) => void; export function useQueryParamState( stateName: string, - debounceTime: number = 100, ): [T | undefined, SetQueryParams] { - const navigate = useNavigate(); const location = useLocation(); const [queryParamState, setQueryParamState] = useState( extractState(location.search, stateName), @@ -83,10 +81,16 @@ export function useQueryParamState( ); if (location.search !== queryString) { - navigate({ ...location, search: `?${queryString}` }, { replace: true }); + // We fallback to the history API, as navigate from react-router causes + // input elements to loose focus. + history.replaceState( + undefined, + document.title, + `${location.pathname}?${queryString}`, + ); } }, - debounceTime, + 100, [queryParamState], ); diff --git a/plugins/api-docs/src/components/ApiExplorerTable/ApiExplorerTable.tsx b/plugins/api-docs/src/components/ApiExplorerTable/ApiExplorerTable.tsx index 39c2caaf1b..5b9808e26d 100644 --- a/plugins/api-docs/src/components/ApiExplorerTable/ApiExplorerTable.tsx +++ b/plugins/api-docs/src/components/ApiExplorerTable/ApiExplorerTable.tsx @@ -156,7 +156,6 @@ export const ApiExplorerTable = ({ }: ExplorerTableProps) => { const [queryParamState, setQueryParamState] = useQueryParamState( 'apiTable', - 500, ); if (error) { From a3b102c827cd7f34525f8f3bfc7004959b4167be Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Wed, 5 May 2021 17:20:03 +0200 Subject: [PATCH 2/5] Use `useSearchParams` in `useQueryParamState` Signed-off-by: Oliver Sand --- packages/core/src/hooks/useQueryParamState.ts | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/packages/core/src/hooks/useQueryParamState.ts b/packages/core/src/hooks/useQueryParamState.ts index 0a61771eba..023d69bf4f 100644 --- a/packages/core/src/hooks/useQueryParamState.ts +++ b/packages/core/src/hooks/useQueryParamState.ts @@ -17,7 +17,7 @@ import { isEqual } from 'lodash'; import qs from 'qs'; import { useEffect, useState } from 'react'; -import { useLocation } from 'react-router-dom'; +import { useSearchParams } from 'react-router-dom'; import { useDebounce } from 'react-use'; function stringify(queryParams: any): string { @@ -59,39 +59,34 @@ type SetQueryParams = (params: T) => void; export function useQueryParamState( stateName: string, ): [T | undefined, SetQueryParams] { - const location = useLocation(); + const [searchParams, setSearchParams] = useSearchParams(); + const searchParamsString = searchParams.toString(); const [queryParamState, setQueryParamState] = useState( - extractState(location.search, stateName), + extractState(searchParamsString, stateName), ); useEffect(() => { - const newState = extractState(location.search, stateName); + const newState = extractState(searchParamsString, stateName); setQueryParamState(oldState => isEqual(newState, oldState) ? oldState : newState, ); - }, [location, stateName]); + }, [searchParamsString, setQueryParamState, stateName]); useDebounce( () => { const queryString = joinQueryString( - location.search, + searchParamsString, stateName, queryParamState, ); - if (location.search !== queryString) { - // We fallback to the history API, as navigate from react-router causes - // input elements to loose focus. - history.replaceState( - undefined, - document.title, - `${location.pathname}?${queryString}`, - ); + if (searchParamsString !== queryString) { + setSearchParams(queryString, { replace: true }); } }, 100, - [queryParamState], + [setSearchParams, queryParamState, searchParamsString, stateName], ); return [queryParamState, setQueryParamState]; From 42e133b6de6451c7819096c34c72319c7826382d Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Thu, 6 May 2021 10:55:48 +0200 Subject: [PATCH 3/5] Fix table toolbar rerendering to often and losing focus Signed-off-by: Oliver Sand --- .changeset/short-bobcats-mate.md | 3 +- packages/core/src/components/Table/Table.tsx | 136 +++++++++++-------- 2 files changed, 81 insertions(+), 58 deletions(-) diff --git a/.changeset/short-bobcats-mate.md b/.changeset/short-bobcats-mate.md index 22aaf11d77..d63e7058a9 100644 --- a/.changeset/short-bobcats-mate.md +++ b/.changeset/short-bobcats-mate.md @@ -3,4 +3,5 @@ '@backstage/plugin-api-docs': patch --- -Fix setting the state with `useQueryParamState` makes inputs lose their focus. +Fix state persisted in the URL make search input in the table toolbar lose their +focus. diff --git a/packages/core/src/components/Table/Table.tsx b/packages/core/src/components/Table/Table.tsx index 2251b8304e..1dd09e7ab8 100644 --- a/packages/core/src/components/Table/Table.tsx +++ b/packages/core/src/components/Table/Table.tsx @@ -20,6 +20,7 @@ import { makeStyles, Typography, useTheme, + withStyles, } from '@material-ui/core'; // Material-table is not using the standard icons available in in material-ui. https://github.com/mbrn/material-table/issues/51 import AddBox from '@material-ui/icons/AddBox'; @@ -49,6 +50,7 @@ import MTable, { } from 'material-table'; import React, { forwardRef, + MutableRefObject, ReactNode, useCallback, useEffect, @@ -100,19 +102,20 @@ function extractValueByField(data: any, field: string): any | undefined { return value; } -const useHeaderStyles = makeStyles(theme => ({ +const StyledMTableHeader = withStyles(theme => ({ header: { padding: theme.spacing(1, 2, 1, 2.5), borderTop: `1px solid ${theme.palette.grey.A100}`, borderBottom: `1px solid ${theme.palette.grey.A100}`, - color: theme.palette.textSubtle, + // withStyles hasn't a generic overload for theme + color: (theme as BackstageTheme).palette.textSubtle, fontWeight: theme.typography.fontWeightBold, position: 'static', wordBreak: 'normal', }, -})); +}))(MTableHeader); -const useToolbarStyles = makeStyles(theme => ({ +const StyledMTableToolbar = withStyles(theme => ({ root: { padding: theme.spacing(3, 0, 2.5, 2.5), }, @@ -124,7 +127,7 @@ const useToolbarStyles = makeStyles(theme => ({ searchField: { paddingRight: theme.spacing(2), }, -})); +}))(MTableToolbar); const useFilterStyles = makeStyles(() => ({ root: { @@ -208,6 +211,59 @@ export interface TableProps onStateChange?: (state: TableState) => any; } +export function TableToolbar(toolbarProps: { + toolbarRef: MutableRefObject; + setSearch: (value: string) => void; + onSearchChanged: (value: string) => void; + toggleFilters: () => void; + hasFilters: boolean; + selectedFiltersLength: number; +}) { + const { + toolbarRef, + setSearch, + hasFilters, + selectedFiltersLength, + toggleFilters, + } = toolbarProps; + const filtersClasses = useFilterStyles(); + const onSearchChanged = useCallback( + (searchText: string) => { + toolbarProps.onSearchChanged(searchText); + setSearch(searchText); + }, + [toolbarProps, setSearch], + ); + + if (hasFilters) { + return ( +
+
+ + + + + Filters ({selectedFiltersLength}) + +
+ +
+ ); + } + + return ( + + ); +} + export function Table({ columns, options, @@ -219,10 +275,7 @@ export function Table({ onStateChange, ...props }: TableProps) { - const headerClasses = useHeaderStyles(); - const toolbarClasses = useToolbarStyles(); const tableClasses = useTableStyles(); - const filtersClasses = useFilterStyles(); const { data, ...propsWithoutData } = props; @@ -230,9 +283,12 @@ export function Table({ const calculatedInitialState = { ...defaultInitialState, ...initialState }; - const [filtersOpen, toggleFilters] = useState( + const [filtersOpen, setFiltersOpen] = useState( calculatedInitialState.filtersOpen, ); + const toggleFilters = useCallback(() => setFiltersOpen(v => !v), [ + setFiltersOpen, + ]); const [selectedFiltersLength, setSelectedFiltersLength] = useState(0); const [tableData, setTableData] = useState(data as any[]); const [selectedFilters, setSelectedFilters] = useState( @@ -379,64 +435,32 @@ export function Table({ })); }; + const hasFilters = !!filters?.length; const Toolbar = useCallback( toolbarProps => { - const onSearchChanged = (searchText: string) => { - toolbarProps.onSearchChanged(searchText); - setSearch(searchText); - }; - - if (filters?.length) { - return ( -
-
- toggleFilters(el => !el)} - aria-label="filter list" - > - - - - Filters ({selectedFiltersLength}) - -
- -
- ); - } - return ( - ); }, - [ - filters?.length, - selectedFiltersLength, - toggleFilters, - toolbarClasses, - filtersClasses, - setSearch, - toolbarRef, - ], + [toggleFilters, hasFilters, selectedFiltersLength, setSearch, toolbarRef], ); + const hasNoRows = typeof data !== 'function' && data.length === 0; + const columnCount = columns.length; const Body = useCallback( bodyProps => { - if (emptyContent && typeof data !== 'function' && data.length === 0) { + if (emptyContent && hasNoRows) { return ( - {emptyContent} + {emptyContent} ); @@ -444,7 +468,7 @@ export function Table({ return ; }, - [data, emptyContent, columns], + [hasNoRows, emptyContent, columnCount], ); return ( @@ -458,9 +482,7 @@ export function Table({ )} components={{ - Header: headerProps => ( - - ), + Header: StyledMTableHeader, Toolbar, Body, }} From 4d9a337a4a4d7a35f7673f17afd8174abc291241 Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Fri, 7 May 2021 11:54:12 +0200 Subject: [PATCH 4/5] Restore `debounceTime` and mark it as deprecated Signed-off-by: Oliver Sand --- packages/core/src/hooks/useQueryParamState.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/src/hooks/useQueryParamState.ts b/packages/core/src/hooks/useQueryParamState.ts index 023d69bf4f..944e73bf39 100644 --- a/packages/core/src/hooks/useQueryParamState.ts +++ b/packages/core/src/hooks/useQueryParamState.ts @@ -58,6 +58,8 @@ type SetQueryParams = (params: T) => void; export function useQueryParamState( stateName: string, + /** @depracated Don't configure a custom debouceTime */ + debounceTime: number = 250, ): [T | undefined, SetQueryParams] { const [searchParams, setSearchParams] = useSearchParams(); const searchParamsString = searchParams.toString(); @@ -85,7 +87,7 @@ export function useQueryParamState( setSearchParams(queryString, { replace: true }); } }, - 100, + debounceTime, [setSearchParams, queryParamState, searchParamsString, stateName], ); From 6f57e2df67494f596dcdaed3f401a2e5487ae4c0 Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Fri, 7 May 2021 15:09:12 +0200 Subject: [PATCH 5/5] Fix typo Signed-off-by: Oliver Sand --- packages/core/src/hooks/useQueryParamState.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/hooks/useQueryParamState.ts b/packages/core/src/hooks/useQueryParamState.ts index 944e73bf39..173993a466 100644 --- a/packages/core/src/hooks/useQueryParamState.ts +++ b/packages/core/src/hooks/useQueryParamState.ts @@ -58,7 +58,7 @@ type SetQueryParams = (params: T) => void; export function useQueryParamState( stateName: string, - /** @depracated Don't configure a custom debouceTime */ + /** @deprecated Don't configure a custom debouceTime */ debounceTime: number = 250, ): [T | undefined, SetQueryParams] { const [searchParams, setSearchParams] = useSearchParams();