From 2c17e5b073ad6fa65825fee105765b626b73ad50 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Mon, 6 Dec 2021 11:33:00 +0000 Subject: [PATCH 01/10] Fix active submenu items Signed-off-by: hiba-aldalaty --- .changeset/eight-insects-kiss.md | 5 +++++ .../src/layout/Sidebar/SidebarSubmenuItem.tsx | 12 +++++++----- 2 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 .changeset/eight-insects-kiss.md diff --git a/.changeset/eight-insects-kiss.md b/.changeset/eight-insects-kiss.md new file mode 100644 index 0000000000..e42a1b7c27 --- /dev/null +++ b/.changeset/eight-insects-kiss.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-components': patch +--- + +Bug fix - items in sidebar submenu are only active when full path is active (including search parameters) diff --git a/packages/core-components/src/layout/Sidebar/SidebarSubmenuItem.tsx b/packages/core-components/src/layout/Sidebar/SidebarSubmenuItem.tsx index 2bb8a13592..4e2fff169c 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarSubmenuItem.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarSubmenuItem.tsx @@ -116,14 +116,13 @@ export type SidebarSubmenuItemProps = { export const SidebarSubmenuItem = (props: SidebarSubmenuItemProps) => { const { title, to, icon: Icon, dropdownItems } = props; const classes = useStyles(); - const { pathname: locationPathname } = useLocation(); - const { pathname: toPathname } = useResolvedPath(to); + const { pathname: locationPathname, search: locationSearch } = useLocation(); + const { pathname: toPathname, search: toSearch } = useResolvedPath(to ?? ''); const { setIsHoveredOn } = useContext(SidebarItemWithSubmenuContext); const closeSubmenu = () => { setIsHoveredOn(false); }; - - let isActive = locationPathname === toPathname; + let isActive = locationPathname === toPathname && toSearch === locationSearch; const [showDropDown, setShowDropDown] = useState(false); const handleClickDropdown = () => { @@ -132,7 +131,10 @@ export const SidebarSubmenuItem = (props: SidebarSubmenuItemProps) => { if (dropdownItems !== undefined) { dropdownItems.some(item => { const resolvedPath = resolvePath(item.to); - isActive = locationPathname === resolvedPath.pathname; + isActive = + locationPathname === resolvedPath.pathname && + locationSearch === toSearch; + return isActive; }); return (
From ec20755eda86b91f59d0db1a893598f7f9a13911 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Mon, 6 Dec 2021 11:45:14 +0000 Subject: [PATCH 02/10] Remove unnecessary ternary operator Signed-off-by: hiba-aldalaty --- .../core-components/src/layout/Sidebar/SidebarSubmenuItem.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core-components/src/layout/Sidebar/SidebarSubmenuItem.tsx b/packages/core-components/src/layout/Sidebar/SidebarSubmenuItem.tsx index 4e2fff169c..3d17a67a49 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarSubmenuItem.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarSubmenuItem.tsx @@ -117,7 +117,7 @@ export const SidebarSubmenuItem = (props: SidebarSubmenuItemProps) => { const { title, to, icon: Icon, dropdownItems } = props; const classes = useStyles(); const { pathname: locationPathname, search: locationSearch } = useLocation(); - const { pathname: toPathname, search: toSearch } = useResolvedPath(to ?? ''); + const { pathname: toPathname, search: toSearch } = useResolvedPath(to); const { setIsHoveredOn } = useContext(SidebarItemWithSubmenuContext); const closeSubmenu = () => { setIsHoveredOn(false); From d76184a53a536a9115419cdcc7293377dc78fc93 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Mon, 6 Dec 2021 11:49:23 +0000 Subject: [PATCH 03/10] Fix changeset Signed-off-by: hiba-aldalaty --- .changeset/eight-insects-kiss.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/eight-insects-kiss.md b/.changeset/eight-insects-kiss.md index e42a1b7c27..d44fbd3577 100644 --- a/.changeset/eight-insects-kiss.md +++ b/.changeset/eight-insects-kiss.md @@ -2,4 +2,4 @@ '@backstage/core-components': patch --- -Bug fix - items in sidebar submenu are only active when full path is active (including search parameters) +Items in are now only active when their full path is active (including search parameters). From f85fb5b140128e054d6afb30451ba6babe2f1288 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Mon, 6 Dec 2021 12:40:53 +0000 Subject: [PATCH 04/10] Update .changeset/eight-insects-kiss.md Signed-off-by: hiba-aldalaty --- .changeset/eight-insects-kiss.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/eight-insects-kiss.md b/.changeset/eight-insects-kiss.md index d44fbd3577..4b09829bf3 100644 --- a/.changeset/eight-insects-kiss.md +++ b/.changeset/eight-insects-kiss.md @@ -2,4 +2,4 @@ '@backstage/core-components': patch --- -Items in are now only active when their full path is active (including search parameters). +Items in `` are now only active when their full path is active (including search parameters). From b3de772e22eeba51cc85cf21056dd48f78f7cc18 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Wed, 8 Dec 2021 12:42:59 +0000 Subject: [PATCH 05/10] Create isLocationMatch function for location matching and setting active sidebar items Signed-off-by: hiba-aldalaty --- .../src/layout/Sidebar/Items.tsx | 11 +++--- .../src/layout/Sidebar/Sidebar.stories.tsx | 16 +------- .../src/layout/Sidebar/SidebarSubmenuItem.tsx | 11 +++--- .../src/layout/Sidebar/utils.ts | 38 +++++++++++++++++++ 4 files changed, 51 insertions(+), 25 deletions(-) create mode 100644 packages/core-components/src/layout/Sidebar/utils.ts diff --git a/packages/core-components/src/layout/Sidebar/Items.tsx b/packages/core-components/src/layout/Sidebar/Items.tsx index a41c3e4bdd..e1dcd4a15e 100644 --- a/packages/core-components/src/layout/Sidebar/Items.tsx +++ b/packages/core-components/src/layout/Sidebar/Items.tsx @@ -46,6 +46,7 @@ import { SidebarItemWithSubmenuContext, } from './config'; import { SidebarSubmenu } from './SidebarSubmenu'; +import { isLocationMatch } from './utils'; export type SidebarItemClassKey = | 'root' @@ -172,7 +173,7 @@ const useStyles = makeStyles( function isSidebarItemWithSubmenuActive( submenu: ReactNode, - locationPathname: string, + currentLocation: any, ) { // Item is active if any of submenu items have active paths const toPathnames: string[] = []; @@ -193,8 +194,8 @@ function isSidebarItemWithSubmenuActive( } }); isActive = toPathnames.some(to => { - const toPathname = resolvePath(to); - return locationPathname === toPathname.pathname; + const toLocation = resolvePath(to); + return isLocationMatch(currentLocation, toLocation); }); return isActive; } @@ -207,8 +208,8 @@ const SidebarItemWithSubmenu = ({ }: PropsWithChildren) => { const classes = useStyles(); const [isHoveredOn, setIsHoveredOn] = useState(false); - const { pathname: locationPathname } = useLocation(); - const isActive = isSidebarItemWithSubmenuActive(children, locationPathname); + const currentLocation = useLocation(); + const isActive = isSidebarItemWithSubmenuActive(children, currentLocation); const handleMouseEnter = () => { setIsHoveredOn(true); diff --git a/packages/core-components/src/layout/Sidebar/Sidebar.stories.tsx b/packages/core-components/src/layout/Sidebar/Sidebar.stories.tsx index 5e68aa8d62..6afd0bf157 100644 --- a/packages/core-components/src/layout/Sidebar/Sidebar.stories.tsx +++ b/packages/core-components/src/layout/Sidebar/Sidebar.stories.tsx @@ -17,8 +17,6 @@ import AddCircleOutlineIcon from '@material-ui/icons/AddCircleOutline'; import HomeOutlinedIcon from '@material-ui/icons/HomeOutlined'; import BuildRoundedIcon from '@material-ui/icons/BuildRounded'; -import LibraryBooksOutlinedIcon from '@material-ui/icons/LibraryBooksOutlined'; -import WebOutlinedIcon from '@material-ui/icons/WebOutlined'; import React from 'react'; import { MemoryRouter } from 'react-router-dom'; import { @@ -34,7 +32,7 @@ import { import { SidebarSubmenuItem } from './SidebarSubmenuItem'; import MenuBookIcon from '@material-ui/icons/MenuBook'; import CloudQueueIcon from '@material-ui/icons/CloudQueue'; -import SettingsApplications from '@material-ui/icons/SettingsApplications'; +import AppsIcon from '@material-ui/icons/Apps'; import AcUnitIcon from '@material-ui/icons/AcUnit'; import { SidebarSubmenu } from './SidebarSubmenu'; @@ -76,17 +74,7 @@ export const SampleScalableSidebar = () => ( - - - + (theme => ({ item: { @@ -116,13 +117,13 @@ export type SidebarSubmenuItemProps = { export const SidebarSubmenuItem = (props: SidebarSubmenuItemProps) => { const { title, to, icon: Icon, dropdownItems } = props; const classes = useStyles(); - const { pathname: locationPathname, search: locationSearch } = useLocation(); - const { pathname: toPathname, search: toSearch } = useResolvedPath(to); const { setIsHoveredOn } = useContext(SidebarItemWithSubmenuContext); const closeSubmenu = () => { setIsHoveredOn(false); }; - let isActive = locationPathname === toPathname && toSearch === locationSearch; + const toLocation = useResolvedPath(to); + const currentLocation = useLocation(); + let isActive = isLocationMatch(currentLocation, toLocation); const [showDropDown, setShowDropDown] = useState(false); const handleClickDropdown = () => { @@ -131,9 +132,7 @@ export const SidebarSubmenuItem = (props: SidebarSubmenuItemProps) => { if (dropdownItems !== undefined) { dropdownItems.some(item => { const resolvedPath = resolvePath(item.to); - isActive = - locationPathname === resolvedPath.pathname && - locationSearch === toSearch; + isActive = isLocationMatch(currentLocation, resolvedPath); return isActive; }); return ( diff --git a/packages/core-components/src/layout/Sidebar/utils.ts b/packages/core-components/src/layout/Sidebar/utils.ts new file mode 100644 index 0000000000..8fefb77bd9 --- /dev/null +++ b/packages/core-components/src/layout/Sidebar/utils.ts @@ -0,0 +1,38 @@ +/* + * Copyright 2020 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Location, State, Path } from 'history'; +import { isEqual, isMatch } from 'lodash'; +import qs from 'qs'; + +export function isLocationMatch( + currentLocation: Location, + toLocation: Path, +) { + const toDecodedSearch = new URLSearchParams(toLocation.search).toString(); + const toQueryParameters = qs.parse(toDecodedSearch); + + const currentDecodedSearch = new URLSearchParams( + currentLocation.search, + ).toString(); + const currentQueryParameters = qs.parse(currentDecodedSearch); + + const matching = + isEqual(toLocation.pathname, currentLocation.pathname) && + isMatch(currentQueryParameters, toQueryParameters); + + return matching; +} From 67688f46016c1435114d6d5ab81fb92b7d10b759 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Wed, 8 Dec 2021 13:11:22 +0000 Subject: [PATCH 06/10] fix generic type error Signed-off-by: hiba-aldalaty --- packages/core-components/src/layout/Sidebar/utils.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/core-components/src/layout/Sidebar/utils.ts b/packages/core-components/src/layout/Sidebar/utils.ts index 8fefb77bd9..1afebe796d 100644 --- a/packages/core-components/src/layout/Sidebar/utils.ts +++ b/packages/core-components/src/layout/Sidebar/utils.ts @@ -14,14 +14,11 @@ * limitations under the License. */ -import { Location, State, Path } from 'history'; +import { Location, Path } from 'history'; import { isEqual, isMatch } from 'lodash'; import qs from 'qs'; -export function isLocationMatch( - currentLocation: Location, - toLocation: Path, -) { +export function isLocationMatch(currentLocation: Location, toLocation: Path) { const toDecodedSearch = new URLSearchParams(toLocation.search).toString(); const toQueryParameters = qs.parse(toDecodedSearch); From f2b4b348e44c676bc1e8174de93fa5e67c63f259 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Fri, 10 Dec 2021 10:20:59 +0000 Subject: [PATCH 07/10] Add tests for isLocationMatch function Signed-off-by: hiba-aldalaty --- packages/app/src/components/Root/Root.tsx | 18 +++- .../src/layout/Sidebar/utils.test.ts | 98 +++++++++++++++++++ 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 packages/core-components/src/layout/Sidebar/utils.test.ts diff --git a/packages/app/src/components/Root/Root.tsx b/packages/app/src/components/Root/Root.tsx index 2d9b9b97f1..40834de76b 100644 --- a/packages/app/src/components/Root/Root.tsx +++ b/packages/app/src/components/Root/Root.tsx @@ -43,6 +43,8 @@ import { SidebarDivider, SidebarSpace, SidebarScrollWrapper, + SidebarSubmenu, + SidebarSubmenuItem, } from '@backstage/core-components'; import { AzurePullRequestsIcon } from '@backstage/plugin-azure-devops'; @@ -81,13 +83,27 @@ const SidebarLogo = () => { export const Root = ({ children }: PropsWithChildren<{}>) => ( - + {/* Global nav, not org-specific */} + + + + + + diff --git a/packages/core-components/src/layout/Sidebar/utils.test.ts b/packages/core-components/src/layout/Sidebar/utils.test.ts new file mode 100644 index 0000000000..b08dcda0fc --- /dev/null +++ b/packages/core-components/src/layout/Sidebar/utils.test.ts @@ -0,0 +1,98 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { Location, Path } from 'history'; +import { isLocationMatch } from './utils'; + +describe('isLocationMatching', () => { + let currentLocation: Location; + let toLocation: Path; + + it('return false when pathname in target and current location differ', async () => { + currentLocation = { + pathname: '/catalog', + search: '?kind=component', + state: null, + hash: '', + key: '', + }; + toLocation = { + pathname: '/catalog-a', + search: '?kind=component', + hash: '', + }; + expect(isLocationMatch(currentLocation, toLocation)).toBe(false); + }); + + it('return true when exact match between current and target location parameters', async () => { + currentLocation = { + pathname: '/catalog', + search: '?kind=component', + state: null, + hash: '', + key: '', + }; + toLocation = { pathname: '/catalog', search: '?kind=component', hash: '' }; + expect(isLocationMatch(currentLocation, toLocation)).toBe(true); + }); + + it('return true when target query parameters are subset of current location query parameters', async () => { + currentLocation = { + pathname: '/catalog', + search: '?x=foo&y=bar', + state: null, + hash: '', + key: '', + }; + toLocation = { pathname: '/catalog', search: '?x=foo', hash: '' }; + expect(isLocationMatch(currentLocation, toLocation)).toBe(true); + }); + + it('return false when no matching query parameters between target and current location', async () => { + currentLocation = { + pathname: '/catalog', + search: '?y=bar', + state: null, + hash: '', + key: '', + }; + toLocation = { pathname: '/catalog', search: '?x=foo', hash: '' }; + expect(isLocationMatch(currentLocation, toLocation)).toBe(false); + }); + + it('return true when query parameters match in different order', async () => { + currentLocation = { + pathname: '/catalog', + search: '?y=bar&x=foo', + state: null, + hash: '', + key: '', + }; + toLocation = { pathname: '/catalog', search: '?x=foo&y=bar', hash: '' }; + expect(isLocationMatch(currentLocation, toLocation)).toBe(true); + }); + + it('return true when there is a matching query parameter alongside extra parameters', async () => { + currentLocation = { + pathname: '/catalog', + search: '?y=bar&x=foo', + state: null, + hash: '', + key: '', + }; + toLocation = { pathname: '/catalog', search: '', hash: '' }; + expect(isLocationMatch(currentLocation, toLocation)).toBe(true); + }); +}); From c13d0018ca20553e27afb90f2f59149bb4cc965d Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Fri, 10 Dec 2021 10:23:59 +0000 Subject: [PATCH 08/10] Undo changes to sidebar in Root.tsx used for testing Signed-off-by: hiba-aldalaty --- packages/app/src/components/Root/Root.tsx | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/packages/app/src/components/Root/Root.tsx b/packages/app/src/components/Root/Root.tsx index 40834de76b..657d64c3ed 100644 --- a/packages/app/src/components/Root/Root.tsx +++ b/packages/app/src/components/Root/Root.tsx @@ -83,27 +83,13 @@ const SidebarLogo = () => { export const Root = ({ children }: PropsWithChildren<{}>) => ( - + {/* Global nav, not org-specific */} - - - - - - From 501eb4ea9f25c77d8bd7f4337cd76076abb3d525 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Fri, 10 Dec 2021 10:24:59 +0000 Subject: [PATCH 09/10] Undo changes to sidebar in Root.tsx used for testing Signed-off-by: hiba-aldalaty --- packages/app/src/components/Root/Root.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/app/src/components/Root/Root.tsx b/packages/app/src/components/Root/Root.tsx index 657d64c3ed..2d9b9b97f1 100644 --- a/packages/app/src/components/Root/Root.tsx +++ b/packages/app/src/components/Root/Root.tsx @@ -43,8 +43,6 @@ import { SidebarDivider, SidebarSpace, SidebarScrollWrapper, - SidebarSubmenu, - SidebarSubmenuItem, } from '@backstage/core-components'; import { AzurePullRequestsIcon } from '@backstage/plugin-azure-devops'; From 519198e2736fdb01bb07d80094bb4dc38ee28980 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Fri, 10 Dec 2021 11:09:04 +0000 Subject: [PATCH 10/10] Update type for parameter in isSidebarItemWithSubmenuActive function Signed-off-by: hiba-aldalaty --- packages/core-components/src/layout/Sidebar/Items.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core-components/src/layout/Sidebar/Items.tsx b/packages/core-components/src/layout/Sidebar/Items.tsx index 84f399ffa7..8b885134df 100644 --- a/packages/core-components/src/layout/Sidebar/Items.tsx +++ b/packages/core-components/src/layout/Sidebar/Items.tsx @@ -47,6 +47,7 @@ import { } from './config'; import { SidebarSubmenu } from './SidebarSubmenu'; import { isLocationMatch } from './utils'; +import { Location } from 'history'; export type SidebarItemClassKey = | 'root' @@ -173,7 +174,7 @@ const useStyles = makeStyles( function isSidebarItemWithSubmenuActive( submenu: ReactNode, - currentLocation: any, + currentLocation: Location, ) { // Item is active if any of submenu items have active paths const toPathnames: string[] = [];