Skip to content

Commit

Permalink
Document outline: Use links not buttons (#10815)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>

* Update packages/editor/src/components/document-outline/test/index.js

remove leading _

Co-Authored-By: adamsilverstein <[email protected]>

* Update packages/editor/src/components/document-outline/index.js

Co-Authored-By: adamsilverstein <[email protected]>

* 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 <[email protected]>

* 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
  • Loading branch information
Adam Silverstein authored Mar 13, 2019
1 parent c7d14ca commit a06f931
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 39 deletions.
29 changes: 8 additions & 21 deletions packages/editor/src/components/document-outline/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand All @@ -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 }
Expand All @@ -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 :
Expand Down Expand Up @@ -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 );
14 changes: 6 additions & 8 deletions packages/editor/src/components/document-outline/item.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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>
{
Expand All @@ -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>
);

Expand Down
4 changes: 4 additions & 0 deletions packages/editor/src/components/document-outline/style.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
display: flex;
margin: 4px 0;

a {
text-decoration: none;
}

.document-outline__emdash::before {
color: $light-gray-500;
margin-right: 4px;
Expand Down
8 changes: 4 additions & 4 deletions packages/editor/src/components/document-outline/test/__snapshots__/index.js.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
15 changes: 12 additions & 3 deletions packages/editor/src/components/document-outline/test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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();
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion packages/editor/src/components/table-of-contents/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function TableOfContents( { hasBlocks, hasOutlineItemsDisabled } ) {
aria-disabled={ ! hasBlocks }
/>
) }
renderContent={ () => <TableOfContentsPanel hasOutlineItemsDisabled={ hasOutlineItemsDisabled } /> }
renderContent={ ( { onClose } ) => <TableOfContentsPanel onRequestClose={ onClose } hasOutlineItemsDisabled={ hasOutlineItemsDisabled } /> }
/>
);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/editor/src/components/table-of-contents/panel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>
Expand Down

0 comments on commit a06f931

Please sign in to comment.