From a95a6154bfa6827f68eb6e04bd09612e15e78778 Mon Sep 17 00:00:00 2001 From: Adam Silverstein Date: Wed, 13 Mar 2019 10:29:56 -0600 Subject: [PATCH] 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 * Update packages/editor/src/components/document-outline/test/index.js remove leading _ Co-Authored-By: adamsilverstein * Update packages/editor/src/components/document-outline/index.js Co-Authored-By: adamsilverstein * 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 * 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 --- .../src/components/document-outline/index.js | 29 +++++-------------- .../src/components/document-outline/item.js | 14 ++++----- .../components/document-outline/style.scss | 4 +++ .../test/__snapshots__/index.js.snap | 8 ++--- .../components/document-outline/test/index.js | 15 ++++++++-- .../src/components/table-of-contents/index.js | 2 +- .../src/components/table-of-contents/panel.js | 4 +-- 7 files changed, 37 insertions(+), 39 deletions(-) diff --git a/packages/editor/src/components/document-outline/index.js b/packages/editor/src/components/document-outline/index.js index b73af4a92d087..5a61503909a88 100644 --- 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 { 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 index c0638100d25f1..1a92e7dbafc4e 100644 --- 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, } ) => (
  • - +
    • ); diff --git a/packages/editor/src/components/document-outline/style.scss b/packages/editor/src/components/document-outline/style.scss index 53008b91e6b3d..c02f815eb6f09 100644 --- 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 index 89d0ebb04d658..8f79943b2f5d9 100644 --- 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`] = ` >
      Heading parent Heading child @@ -33,10 +33,10 @@ exports[`DocumentOutline header blocks present should render warnings for multip >
        Heading 1 @@ -50,10 +50,10 @@ exports[`DocumentOutline header blocks present should render warnings for multip Heading 1 diff --git a/packages/editor/src/components/document-outline/test/index.js b/packages/editor/src/components/document-outline/test/index.js index 024cb615beeeb..5ee66565d2dad 100644 --- 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( ); 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( ); 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( ); 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 index 767f51dc6c1af..b027f6c09eefc 100644 --- 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={ () => } + renderContent={ ( { onClose } ) => } /> ); } diff --git a/packages/editor/src/components/table-of-contents/panel.js b/packages/editor/src/components/table-of-contents/panel.js index a1a2a4b413055..901c2a31556e0 100644 --- 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 (
        { __( 'Document Outline' ) } - + ) }