Sidebar refactoring

personalizedrefrigerator · Oct 7, 2024 · 51f8e52 · 51f8e52
1 parent d1ac3d4
commit 51f8e52
Show file tree

Hide file tree

Showing 16 changed files with 161 additions and 87 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -429,6 +429,7 @@ packages/app-desktop/gui/Sidebar/listItemComponents/ExpandIcon.js
 packages/app-desktop/gui/Sidebar/listItemComponents/ExpandLink.js
 packages/app-desktop/gui/Sidebar/listItemComponents/FolderItem.js
 packages/app-desktop/gui/Sidebar/listItemComponents/HeaderItem.js
+packages/app-desktop/gui/Sidebar/listItemComponents/ListItemWrapper.js
 packages/app-desktop/gui/Sidebar/listItemComponents/NoteCount.js
 packages/app-desktop/gui/Sidebar/listItemComponents/TagItem.js
 packages/app-desktop/gui/Sidebar/styles/index.js

diff --git a/.gitignore b/.gitignore
@@ -406,6 +406,7 @@ packages/app-desktop/gui/Sidebar/listItemComponents/ExpandIcon.js
 packages/app-desktop/gui/Sidebar/listItemComponents/ExpandLink.js
 packages/app-desktop/gui/Sidebar/listItemComponents/FolderItem.js
 packages/app-desktop/gui/Sidebar/listItemComponents/HeaderItem.js
+packages/app-desktop/gui/Sidebar/listItemComponents/ListItemWrapper.js
 packages/app-desktop/gui/Sidebar/listItemComponents/NoteCount.js
 packages/app-desktop/gui/Sidebar/listItemComponents/TagItem.js
 packages/app-desktop/gui/Sidebar/styles/index.js

diff --git a/packages/app-desktop/gui/ItemList.tsx b/packages/app-desktop/gui/ItemList.tsx
@@ -14,6 +14,7 @@ interface Props<ItemType> {
 	id?: string;
 	role?: string;
 	'aria-label'?: string;
+	tabIndex?: number;
 }
 
 interface State {
@@ -177,6 +178,7 @@ class ItemList<ItemType> extends React.Component<Props<ItemType>, State> {
 				role={this.props.role}
 				aria-label={this.props['aria-label']}
 				aria-setsize={items.length}
+				tabIndex={this.props.tabIndex}
 
 				onScroll={this.onScroll}
 				onKeyDown={this.onKeyDown}

diff --git a/packages/app-desktop/gui/Sidebar/FolderAndTagList.tsx b/packages/app-desktop/gui/Sidebar/FolderAndTagList.tsx
@@ -44,6 +44,7 @@ const FolderAndTagList: React.FC<Props> = props => {
 		...props,
 		selectedIndex,
 		onSelectedElementShown: setSelectedListElement,
+		listItems,
 	});
 
 	const onKeyEventHandler = useOnSidebarKeyDownHandler({
@@ -75,6 +76,8 @@ const FolderAndTagList: React.FC<Props> = props => {
 				items={listItems}
 				itemRenderer={onRenderItem}
 				onKeyDown={onKeyEventHandler}
+				tabIndex={0}
+				role='tree'
 
 				itemHeight={30}
 			/>

diff --git a/packages/app-desktop/gui/Sidebar/hooks/useFocusHandler.ts b/packages/app-desktop/gui/Sidebar/hooks/useFocusHandler.ts
@@ -1,4 +1,4 @@
-import { MutableRefObject, RefObject, useCallback, useEffect, useMemo, useRef } from 'react';
+import { RefObject, useCallback, useEffect, useMemo, useRef } from 'react';
 import { ListItem } from '../types';
 import ItemList from '../../ItemList';
 import { focus } from '@joplin/lib/utils/focusHandler';
@@ -10,20 +10,8 @@ interface Props {
 	listItems: ListItem[];
 }
 
-const useFocusAfterNextRenderHandler = (
-	shouldFocusAfterNextRender: MutableRefObject<boolean>,
-	selectedListElement: HTMLElement|null,
-) => {
-	useEffect(() => {
-		if (!shouldFocusAfterNextRender.current || !selectedListElement) return;
-		focus('FolderAndTagList/useFocusHandler/afterRender', selectedListElement);
-		shouldFocusAfterNextRender.current = false;
-	}, [selectedListElement, shouldFocusAfterNextRender]);
-};
-
-const useRefocusOnSelectionChangeHandler = (
+const useScrollToSelectionHandler = (
 	itemListRef: RefObject<ItemList<ListItem>>,
-	shouldFocusAfterNextRender: MutableRefObject<boolean>,
 	listItems: ListItem[],
 	selectedIndex: number,
 ) => {
@@ -49,31 +37,24 @@ const useRefocusOnSelectionChangeHandler = (
 	useEffect(() => {
 		if (!itemListRef.current || !selectedItemKey) return;
 
-		const hasFocus = !!itemListRef.current.container.querySelector(':scope :focus');
-		shouldFocusAfterNextRender.current = hasFocus;
+		const hasFocus = !!itemListRef.current.container.contains(document.activeElement);
 
 		if (hasFocus) {
 			itemListRef.current.makeItemIndexVisible(selectedIndexRef.current);
 		}
-	}, [selectedItemKey, itemListRef, shouldFocusAfterNextRender]);
+	}, [selectedItemKey, itemListRef]);
 };
 
 const useFocusHandler = (props: Props) => {
 	const { itemListRef, selectedListElement, selectedIndex, listItems } = props;
 
-	// When set to true, when selectedListElement next changes, select it.
-	const shouldFocusAfterNextRender = useRef(false);
-
-	useRefocusOnSelectionChangeHandler(itemListRef, shouldFocusAfterNextRender, listItems, selectedIndex);
-	useFocusAfterNextRenderHandler(shouldFocusAfterNextRender, selectedListElement);
+	useScrollToSelectionHandler(itemListRef, listItems, selectedIndex);
 
 	const focusSidebar = useCallback(() => {
 		if (!selectedListElement || !itemListRef.current.isIndexVisible(selectedIndex)) {
 			itemListRef.current.makeItemIndexVisible(selectedIndex);
-			shouldFocusAfterNextRender.current = true;
-		} else {
-			focus('FolderAndTagList/useFocusHandler/focusSidebar', selectedListElement);
 		}
+		focus('FolderAndTagList/useFocusHandler/focusSidebar', itemListRef.current.container);
 	}, [selectedListElement, selectedIndex, itemListRef]);
 
 	return { focusSidebar };

diff --git a/packages/app-desktop/gui/Sidebar/hooks/useOnRenderItem.tsx b/packages/app-desktop/gui/Sidebar/hooks/useOnRenderItem.tsx
@@ -29,6 +29,7 @@ import Logger from '@joplin/utils/Logger';
 import onFolderDrop from '@joplin/lib/models/utils/onFolderDrop';
 import HeaderItem from '../listItemComponents/HeaderItem';
 import AllNotesItem from '../listItemComponents/AllNotesItem';
+import ListItemWrapper from '../listItemComponents/ListItemWrapper';
 
 const Menu = bridge().Menu;
 const MenuItem = bridge().MenuItem;
@@ -44,6 +45,7 @@ interface Props {
 
 	selectedIndex: number;
 	onSelectedElementShown: (element: HTMLElement)=> void;
+	listItems: ListItem[];
 }
 
 type ItemContextMenuListener = MouseEventHandler<HTMLElement>;
@@ -326,26 +328,21 @@ const useOnRenderItem = (props: Props) => {
 	const selectedIndexRef = useRef(props.selectedIndex);
 	selectedIndexRef.current = props.selectedIndex;
 
+	const itemCount = props.listItems.length;
 	return useCallback((item: ListItem, index: number) => {
 		const selected = props.selectedIndex === index;
-		const anchorRefCallback = selected ? (
-			(element: HTMLElement) => {
-				if (selectedIndexRef.current === index) {
-					props.onSelectedElementShown(element);
-				}
-			}
-		) : null;
 
 		if (item.kind === ListItemType.Tag) {
 			const tag = item.tag;
 			return <TagItem
 				key={item.key}
-				anchorRef={anchorRefCallback}
 				selected={selected}
 				onClick={tagItem_click}
 				onTagDrop={onTagDrop_}
 				onContextMenu={onItemContextMenu}
 				tag={tag}
+				itemCount={itemCount}
+				index={index}
 			/>;
 		} else if (item.kind === ListItemType.Folder) {
 			const folder = item.folder;
@@ -368,7 +365,6 @@ const useOnRenderItem = (props: Props) => {
 			}
 			return <FolderItem
 				key={item.key}
-				anchorRef={anchorRefCallback}
 				selected={selected}
 				folderId={folder.id}
 				folderTitle={Folder.displayTitle(folder)}
@@ -386,23 +382,36 @@ const useOnRenderItem = (props: Props) => {
 				shareId={folder.share_id}
 				parentId={folder.parent_id}
 				showFolderIcon={showFolderIcons}
+				index={index}
+				itemCount={itemCount}
 			/>;
 		} else if (item.kind === ListItemType.Header) {
 			return <HeaderItem
 				key={item.id}
 				item={item}
-				anchorRef={anchorRefCallback}
+				isSelected={selected}
 				onDrop={item.supportsFolderDrop ? onFolderDrop_ : null}
+				index={index}
+				itemCount={itemCount}
 			/>;
 		} else if (item.kind === ListItemType.AllNotes) {
 			return <AllNotesItem
 				key={item.key}
 				selected={selected}
-				anchorRef={anchorRefCallback}
+				index={index}
+				itemCount={itemCount}
 			/>;
 		} else if (item.kind === ListItemType.Spacer) {
 			return (
-				<a key={item.key} className='sidebar-spacer-item' ref={anchorRefCallback} aria-label={_('Spacer')}></a>
+				<ListItemWrapper
+					key={item.key}
+					selected={selected}
+					itemIndex={index}
+					itemCount={itemCount}
+					className='sidebar-spacer-item'
+				>
+					<div aria-label={_('Spacer')}></div>
+				</ListItemWrapper>
 			);
 		} else {
 			const exhaustivenessCheck: never = item;
@@ -422,6 +431,7 @@ const useOnRenderItem = (props: Props) => {
 		tagItem_click,
 		props.selectedIndex,
 		props.onSelectedElementShown,
+		itemCount,
 	]);
 };
 

diff --git a/packages/app-desktop/gui/Sidebar/hooks/useOnSidebarKeyDownHandler.ts b/packages/app-desktop/gui/Sidebar/hooks/useOnSidebarKeyDownHandler.ts
@@ -29,25 +29,22 @@ const useOnSidebarKeyDownHandler = (props: Props) => {
 
 	return useCallback<KeyboardEventHandler<HTMLElement>>((event) => {
 		const selectedItem = listItems[selectedIndex];
+		let indexChange = 0;
+
 		if (selectedItem?.kind === ListItemType.Folder && isToggleShortcut(event.code, selectedItem, collapsedFolderIds)) {
 			event.preventDefault();
 
 			dispatch({
 				type: 'FOLDER_TOGGLE',
 				id: selectedItem.folder.id,
 			});
-		}
-
-		if ((event.ctrlKey || event.metaKey) && event.code === 'KeyA') { // ctrl+a or cmd+a
+		} else if ((event.ctrlKey || event.metaKey) && event.code === 'KeyA') { // ctrl+a or cmd+a
 			event.preventDefault();
-		}
-
-		let indexChange = 0;
-		if (event.code === 'ArrowUp') {
+		} else if (event.code === 'ArrowUp') {
 			indexChange = -1;
 		} else if (event.code === 'ArrowDown') {
 			indexChange = 1;
-		} else if (event.code === 'Tab') {
+		} else if (event.code === 'ArrowRight') {
 			event.preventDefault();
 
 			if (event.shiftKey) {

diff --git a/packages/app-desktop/gui/Sidebar/listItemComponents/AllNotesItem.tsx b/packages/app-desktop/gui/Sidebar/listItemComponents/AllNotesItem.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { StyledAllNotesIcon, StyledListItem, StyledListItemAnchor } from '../styles';
+import { StyledAllNotesIcon, StyledListItemAnchor } from '../styles';
 import { useCallback } from 'react';
 import { Dispatch } from 'redux';
 import bridge from '../../../services/bridge';
@@ -10,6 +10,7 @@ import PerFolderSortOrderService from '../../../services/sortOrder/PerFolderSort
 import { _ } from '@joplin/lib/locale';
 import { connect } from 'react-redux';
 import EmptyExpandLink from './EmptyExpandLink';
+import ListItemWrapper from './ListItemWrapper';
 const { ALL_NOTES_FILTER_ID } = require('@joplin/lib/reserved-ids');
 
 const Menu = bridge().Menu;
@@ -18,7 +19,8 @@ const MenuItem = bridge().MenuItem;
 interface Props {
 	dispatch: Dispatch;
 	selected: boolean;
-	anchorRef: React.Ref<HTMLAnchorElement>;
+	index: number;
+	itemCount: number;
 }
 
 const menuUtils = new MenuUtils(CommandService.instance());
@@ -46,21 +48,25 @@ const AllNotesItem: React.FC<Props> = props => {
 	}, []);
 
 	return (
-		<StyledListItem key="allNotesHeader" selected={props.selected} className={'list-item-container list-item-depth-0 all-notes'} isSpecialItem={true}>
+		<ListItemWrapper
+			key="allNotesHeader"
+			selected={props.selected}
+			className={'list-item-container list-item-depth-0 all-notes'}
+			itemIndex={props.index}
+			itemCount={props.itemCount}
+		>
 			<EmptyExpandLink/>
 			<StyledAllNotesIcon className="icon-notes"/>
 			<StyledListItemAnchor
-				ref={props.anchorRef}
 				className="list-item"
 				isSpecialItem={true}
-				href="#"
 				selected={props.selected}
 				onClick={onAllNotesClick_}
 				onContextMenu={toggleAllNotesContextMenu}
 			>
 				{_('All notes')}
 			</StyledListItemAnchor>
-		</StyledListItem>
+		</ListItemWrapper>
 	);
 };
 

diff --git a/packages/app-desktop/gui/Sidebar/listItemComponents/ExpandLink.tsx b/packages/app-desktop/gui/Sidebar/listItemComponents/ExpandLink.tsx
@@ -13,7 +13,7 @@ interface ExpandLinkProps {
 
 const ExpandLink: React.FC<ExpandLinkProps> = props => {
 	return props.hasChildren ? (
-		<a className='sidebar-expand-link' href="#" data-folder-id={props.folderId} onClick={props.onClick}>
+		<a className='sidebar-expand-link' data-folder-id={props.folderId} onClick={props.onClick}>
 			<ExpandIcon isVisible={true} isExpanded={props.isExpanded} targetTitle={props.folderTitle}/>
 		</a>
 	) : (

diff --git a/packages/app-desktop/gui/Sidebar/listItemComponents/FolderItem.tsx b/packages/app-desktop/gui/Sidebar/listItemComponents/FolderItem.tsx
@@ -2,14 +2,15 @@ import * as React from 'react';
 
 import { FolderIcon, FolderIconType } from '@joplin/lib/services/database/types';
 import ExpandLink from './ExpandLink';
-import { StyledListItem, StyledListItemAnchor, StyledShareIcon, StyledSpanFix } from '../styles';
+import { StyledListItemAnchor, StyledShareIcon, StyledSpanFix } from '../styles';
 import { ItemClickListener, ItemContextMenuListener, ItemDragListener } from '../types';
 import FolderIconBox from '../../FolderIconBox';
 import { getTrashFolderIcon, getTrashFolderId } from '@joplin/lib/services/trash';
 import Folder from '@joplin/lib/models/Folder';
 import { ModelType } from '@joplin/lib/BaseModel';
 import { _ } from '@joplin/lib/locale';
 import NoteCount from './NoteCount';
+import ListItemWrapper from './ListItemWrapper';
 
 const renderFolderIcon = (folderIcon: FolderIcon) => {
 	if (!folderIcon) {
@@ -43,7 +44,9 @@ interface FolderItemProps {
 	onFolderToggleClick_: ItemClickListener;
 	shareId: string;
 	selected: boolean;
-	anchorRef: React.Ref<HTMLElement>;
+
+	index: number;
+	itemCount: number;
 }
 
 function FolderItem(props: FolderItemProps) {
@@ -63,29 +66,39 @@ function FolderItem(props: FolderItemProps) {
 	};
 
 	return (
-		<StyledListItem depth={depth} selected={selected} className={`list-item-container list-item-depth-${depth} ${selected ? 'selected' : ''}`} onDragStart={onFolderDragStart_} onDragOver={onFolderDragOver_} onDrop={onFolderDrop_} draggable={draggable} data-folder-id={folderId}>
+		<ListItemWrapper
+			depth={depth}
+			selected={selected}
+			itemIndex={props.index}
+			itemCount={props.itemCount}
+			aria-expanded={hasChildren ? props.isExpanded : undefined}
+			className={`list-item-container list-item-depth-${depth} ${selected ? 'selected' : ''}`}
+			onDragStart={onFolderDragStart_}
+			onDragOver={onFolderDragOver_}
+			onDrop={onFolderDrop_}
+			draggable={draggable}
+			data-folder-id={folderId}
+		>
 			<ExpandLink hasChildren={hasChildren} folderTitle={folderTitle} folderId={folderId} onClick={onFolderToggleClick_} isExpanded={isExpanded}/>
 			<StyledListItemAnchor
-				ref={props.anchorRef}
 				className="list-item"
 				isConflictFolder={folderId === Folder.conflictFolderId()}
-				href="#"
 				selected={selected}
-				aria-selected={selected}
 				shareId={shareId}
 				data-id={folderId}
 				data-type={ModelType.Folder}
 				onContextMenu={itemContextMenu}
 				data-folder-id={folderId}
+				onDoubleClick={onFolderToggleClick_}
+
 				onClick={() => {
 					folderItem_click(folderId);
 				}}
-				onDoubleClick={onFolderToggleClick_}
 			>
 				{doRenderFolderIcon()}<StyledSpanFix className="title">{folderTitle}</StyledSpanFix>
 				{shareIcon} <NoteCount count={noteCount}/>
 			</StyledListItemAnchor>
-		</StyledListItem>
+		</ListItemWrapper>
 	);
 }