Document outline: Use links not buttons (#10815)

* Adjust document outline to use an a tag vs button * target href links directly to page anchors, remove onClick handler * update test snapshot * update snapshot * better titleNode targeting * update snapshot * update snapshot * Close the table of contents panel when a link is clicked * add deterministic block id to tests * remove redundant screen reader text * Adjust map to avoid mutating original object * update snapshot * Update packages/editor/src/components/document-outline/index.js Co-Authored-By: adamsilverstein <adam@10up.com> * Update packages/editor/src/components/document-outline/test/index.js remove leading _ Co-Authored-By: adamsilverstein <adam@10up.com> * Update packages/editor/src/components/document-outline/index.js Co-Authored-By: adamsilverstein <adam@10up.com> * update snapshot * update snapshots * update snapshot * update snapshots * fix up e2e tests * Fix snapshots by removing single quotes from outline links * Update packages/editor/src/components/document-outline/index.js Co-Authored-By: adamsilverstein <adam@10up.com> * change target to href * rename close -> closeOutline * update snapshots after property name changes * rename close/onClose -> onRequestClose for TOC, on * restore onSelect * complete renaming * Block links are only valid for the current session - remove hash after following * update snapshot * cleanup; move block id hash removal functionality up to document outline; now includes title * update snapshot * use replaceState vs pushState * use defer and import at top of file * removeURLHash as helper * remove passing event in select handler * improve doc block * Skip title in outline when title node not found * remove removeURLHash
WordPress · Mar 20, 2019 · a95a615 · a95a615
1 parent 336a1a4
commit a95a615
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 39 deletions.
diff --git a/packages/editor/src/components/document-outline/index.js b/packages/editor/src/components/document-outline/index.js
@@ -8,7 +8,7 @@ import { countBy, flatMap, get } from 'lodash';
  */
 import { __ } from '@wordpress/i18n';
 import { compose } from '@wordpress/compose';
-import { withSelect, withDispatch } from '@wordpress/data';
+import { withSelect } from '@wordpress/data';
 import {
 	create,
 	getTextContent,
@@ -73,18 +73,9 @@ export const DocumentOutline = ( { blocks = [], title, onSelect, isTitleSupporte
 
 	let prevHeadingLevel = 1;
 
-	// Select the corresponding block in the main editor
-	// when clicking on a heading item from the list.
-	const onSelectHeading = ( clientId ) => onSelect( clientId );
-	const focusTitle = () => {
-		// Not great but it's the simplest way to focus the title right now.
-		const titleNode = document.querySelector( '.editor-post-title__input' );
-		if ( titleNode ) {
-			titleNode.focus();
-		}
-	};
-
-	const hasTitle = isTitleSupported && title;
+	// Not great but it's the simplest way to locate the title right now.
+	const titleNode = document.querySelector( '.editor-post-title__input' );
+	const hasTitle = isTitleSupported && title && titleNode;
 	const countByLevel = countBy( headings, 'level' );
 	const hasMultipleH1 = countByLevel[ 1 ] > 1;
 
@@ -95,7 +86,8 @@ export const DocumentOutline = ( { blocks = [], title, onSelect, isTitleSupporte
 					<DocumentOutlineItem
 						level={ __( 'Title' ) }
 						isValid
-						onClick={ focusTitle }
+						onSelect={ onSelect }
+						href={ `#${ titleNode.id }` }
 						isDisabled={ hasOutlineItemsDisabled }
 					>
 						{ title }
@@ -119,9 +111,10 @@ export const DocumentOutline = ( { blocks = [], title, onSelect, isTitleSupporte
 							key={ index }
 							level={ `H${ item.level }` }
 							isValid={ isValid }
-							onClick={ () => onSelectHeading( item.clientId ) }
 							path={ item.path }
 							isDisabled={ hasOutlineItemsDisabled }
+							href={ `#block-${ item.clientId }` }
+							onSelect={ onSelect }
 						>
 							{ item.isEmpty ?
 								emptyHeadingContent :
@@ -152,11 +145,5 @@ export default compose(
 			blocks: getBlocks(),
 			isTitleSupported: get( postType, [ 'supports', 'title' ], false ),
 		};
-	} ),
-	withDispatch( ( dispatch ) => {
-		const { selectBlock } = dispatch( 'core/block-editor' );
-		return {
-			onSelect: selectBlock,
-		};
 	} )
 )( DocumentOutline );
diff --git a/packages/editor/src/components/document-outline/item.js b/packages/editor/src/components/document-outline/item.js
@@ -6,16 +6,15 @@ import classnames from 'classnames';
 /**
  * WordPress dependencies
  */
-import { __ } from '@wordpress/i18n';
 import { BlockTitle } from '@wordpress/block-editor';
 
 const TableOfContentsItem = ( {
 	children,
 	isValid,
 	level,
-	onClick,
-	isDisabled,
 	path = [],
+	href,
+	onSelect,
 } ) => (
 	<li
 		className={ classnames(
@@ -26,10 +25,10 @@ const TableOfContentsItem = ( {
 			}
 		) }
 	>
-		<button
+		<a
+			href={ href }
 			className="document-outline__button"
-			onClick={ isDisabled ? undefined : onClick }
-			disabled={ isDisabled }
+			onClick={ onSelect }
 		>
 			<span className="document-outline__emdash" aria-hidden="true"></span>
 			{
@@ -47,8 +46,7 @@ const TableOfContentsItem = ( {
 			<span className="document-outline__item-content">
 				{ children }
 			</span>
-			{ ! isDisabled && <span className="screen-reader-text">{ __( '(Click to focus this heading)' ) }</span> }
-		</button>
+		</a>
 	</li>
 );
 

diff --git a/packages/editor/src/components/document-outline/style.scss b/packages/editor/src/components/document-outline/style.scss
@@ -11,6 +11,10 @@
 	display: flex;
 	margin: 4px 0;
 
+	a {
+		text-decoration: none;
+	}
+
 	.document-outline__emdash::before {
 		color: $light-gray-500;
 		margin-right: 4px;

diff --git a/packages/editor/src/components/document-outline/test/__snapshots__/index.js.snap b/packages/editor/src/components/document-outline/test/__snapshots__/index.js.snap
@@ -6,19 +6,19 @@ exports[`DocumentOutline header blocks present should match snapshot 1`] = `
 >
   <ul>
     <TableOfContentsItem
+      href="#block-clientId_0"
       isValid={true}
       key="0"
       level="H2"
-      onClick={[Function]}
       path={Array []}
     >
       Heading parent
     </TableOfContentsItem>
     <TableOfContentsItem
+      href="#block-clientId_1"
       isValid={true}
       key="1"
       level="H3"
-      onClick={[Function]}
       path={Array []}
     >
       Heading child
@@ -33,10 +33,10 @@ exports[`DocumentOutline header blocks present should render warnings for multip
 >
   <ul>
     <TableOfContentsItem
+      href="#block-clientId_0"
       isValid={false}
       key="0"
       level="H1"
-      onClick={[Function]}
       path={Array []}
     >
       Heading 1
@@ -50,10 +50,10 @@ exports[`DocumentOutline header blocks present should render warnings for multip
       </em>
     </TableOfContentsItem>
     <TableOfContentsItem
+      href="#block-clientId_2"
       isValid={false}
       key="1"
       level="H1"
-      onClick={[Function]}
       path={Array []}
     >
       Heading 1

diff --git a/packages/editor/src/components/document-outline/test/index.js b/packages/editor/src/components/document-outline/test/index.js
@@ -80,7 +80,10 @@ describe( 'DocumentOutline', () => {
 		} );
 
 		it( 'should not render when no heading blocks provided', () => {
-			const blocks = [ paragraph ];
+			const blocks = [ paragraph ].map( ( block, index ) => {
+				// Set client IDs to a predictable value.
+				return { ...block, clientId: `clientId_${ index }` };
+			} );
 			const wrapper = shallow( <DocumentOutline blocks={ blocks } /> );
 
 			expect( wrapper.html() ).toBe( null );
@@ -89,7 +92,10 @@ describe( 'DocumentOutline', () => {
 
 	describe( 'header blocks present', () => {
 		it( 'should match snapshot', () => {
-			const blocks = [ headingParent, headingChild ];
+			const blocks = [ headingParent, headingChild ].map( ( block, index ) => {
+				// Set client IDs to a predictable value.
+				return { ...block, clientId: `clientId_${ index }` };
+			} );
 			const wrapper = shallow( <DocumentOutline blocks={ blocks } /> );
 
 			expect( wrapper ).toMatchSnapshot();
@@ -110,7 +116,10 @@ describe( 'DocumentOutline', () => {
 		} );
 
 		it( 'should render warnings for multiple h1 headings', () => {
-			const blocks = [ headingH1, paragraph, headingH1, paragraph ];
+			const blocks = [ headingH1, paragraph, headingH1, paragraph ].map( ( block, index ) => {
+				// Set client IDs to a predictable value.
+				return { ...block, clientId: `clientId_${ index }` };
+			} );
 			const wrapper = shallow( <DocumentOutline blocks={ blocks } /> );
 
 			expect( wrapper ).toMatchSnapshot();

diff --git a/packages/editor/src/components/table-of-contents/index.js b/packages/editor/src/components/table-of-contents/index.js
@@ -26,7 +26,7 @@ function TableOfContents( { hasBlocks, hasOutlineItemsDisabled } ) {
 					aria-disabled={ ! hasBlocks }
 				/>
 			) }
-			renderContent={ () => <TableOfContentsPanel hasOutlineItemsDisabled={ hasOutlineItemsDisabled } /> }
+			renderContent={ ( { onClose } ) => <TableOfContentsPanel onRequestClose={ onClose } hasOutlineItemsDisabled={ hasOutlineItemsDisabled } /> }
 		/>
 	);
 }

diff --git a/packages/editor/src/components/table-of-contents/panel.js b/packages/editor/src/components/table-of-contents/panel.js
@@ -11,7 +11,7 @@ import { withSelect } from '@wordpress/data';
 import WordCount from '../word-count';
 import DocumentOutline from '../document-outline';
 
-function TableOfContentsPanel( { headingCount, paragraphCount, numberOfBlocks, hasOutlineItemsDisabled } ) {
+function TableOfContentsPanel( { headingCount, paragraphCount, numberOfBlocks, hasOutlineItemsDisabled, onRequestClose } ) {
 	return (
 		<Fragment>
 			<div
@@ -49,7 +49,7 @@ function TableOfContentsPanel( { headingCount, paragraphCount, numberOfBlocks, h
 					<span className="table-of-contents__title">
 						{ __( 'Document Outline' ) }
 					</span>
-					<DocumentOutline hasOutlineItemsDisabled={ hasOutlineItemsDisabled } />
+					<DocumentOutline onSelect={ onRequestClose } hasOutlineItemsDisabled={ hasOutlineItemsDisabled } />
 				</Fragment>
 			) }
 		</Fragment>