Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't show heading ancestor blocks in Document Outline. #25599

Merged
merged 2 commits into from
Sep 24, 2020
Merged

Don't show heading ancestor blocks in Document Outline. #25599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f59b93
Don't show heading ancestor blocks in Document Outline.
ZebulanStanphill Sep 24, 2020
58fd29c
Fix unit test.
ZebulanStanphill Sep 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions packages/editor/src/components/document-outline/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,27 +39,23 @@ const multipleH1Headings = [

/**
* Returns an array of heading blocks enhanced with the following properties:
* path - An array of blocks that are ancestors of the heading starting from a top-level node.
* Can be an empty array if the heading is a top-level node (is not nested inside another block).
* level - An integer with the heading level.
* isEmpty - Flag indicating if the heading has no content.
*
* @param {?Array} blocks An array of blocks.
* @param {?Array} path An array of blocks that are ancestors of the blocks passed as blocks.
*
* @return {Array} An array of heading blocks enhanced with the properties described above.
*/
const computeOutlineHeadings = ( blocks = [], path = [] ) => {
const computeOutlineHeadings = ( blocks = [] ) => {
return flatMap( blocks, ( block = {} ) => {
if ( block.name === 'core/heading' ) {
return {
...block,
path,
level: block.attributes.level,
isEmpty: isEmptyHeading( block ),
};
}
return computeOutlineHeadings( block.innerBlocks, [ ...path, block ] );
return computeOutlineHeadings( block.innerBlocks );
} );
};

Expand Down Expand Up @@ -119,7 +115,6 @@ export const DocumentOutline = ( {
key={ index }
level={ `H${ item.level }` }
isValid={ isValid }
path={ item.path }
isDisabled={ hasOutlineItemsDisabled }
href={ `#block-${ item.clientId }` }
onSelect={ onSelect }
Expand Down
15 changes: 0 additions & 15 deletions packages/editor/src/components/document-outline/item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,10 @@
*/
import classnames from 'classnames';

/**
* WordPress dependencies
*/
import { BlockTitle } from '@wordpress/block-editor';

const TableOfContentsItem = ( {
children,
isValid,
level,
path = [],
href,
onSelect,
} ) => (
Expand All @@ -34,15 +28,6 @@ const TableOfContentsItem = ( {
className="document-outline__emdash"
aria-hidden="true"
></span>
{
// path is an array of nodes that are ancestors of the heading starting in the top level node.
// This mapping renders each ancestor to make it easier for the user to know where the headings are nested.
path.map( ( { clientId }, index ) => (
<strong key={ index } className="document-outline__level">
<BlockTitle clientId={ clientId } />
</strong>
) )
}
<strong className="document-outline__level">{ level }</strong>
<span className="document-outline__item-content">{ children }</span>
</a>
Expand Down
8 changes: 2 additions & 6 deletions packages/editor/src/components/document-outline/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,16 @@ exports[`DocumentOutline header blocks present should match snapshot 1`] = `
isValid={true}
key="0"
level="H2"
path={Array []}
>
Heading parent
Heading 2
</TableOfContentsItem>
<TableOfContentsItem
href="#block-clientId_1"
isValid={true}
key="1"
level="H3"
path={Array []}
>
Heading child
Heading 3
</TableOfContentsItem>
</ul>
</div>
Expand All @@ -37,7 +35,6 @@ exports[`DocumentOutline header blocks present should render warnings for multip
isValid={false}
key="0"
level="H1"
path={Array []}
>
Heading 1
<br
Expand All @@ -54,7 +51,6 @@ exports[`DocumentOutline header blocks present should render warnings for multip
isValid={false}
key="1"
level="H1"
path={Array []}
>
Heading 1
<br
Expand Down
48 changes: 21 additions & 27 deletions packages/editor/src/components/document-outline/test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jest.mock( '@wordpress/block-editor', () => ( {
} ) );

describe( 'DocumentOutline', () => {
let paragraph, headingH1, headingParent, headingChild, nestedHeading;
let paragraph, headingH1, headingH2, headingH3, nestedHeading;
beforeAll( () => {
registerBlockType( 'core/heading', {
category: 'text',
Expand Down Expand Up @@ -59,18 +59,15 @@ describe( 'DocumentOutline', () => {
content: 'Heading 1',
level: 1,
} );
headingParent = createBlock( 'core/heading', {
content: 'Heading parent',
headingH2 = createBlock( 'core/heading', {
content: 'Heading 2',
level: 2,
} );
headingChild = createBlock( 'core/heading', {
content: 'Heading child',
headingH3 = createBlock( 'core/heading', {
content: 'Heading 3',
level: 3,
} );

nestedHeading = createBlock( 'core/columns', undefined, [
headingChild,
] );
nestedHeading = createBlock( 'core/columns', undefined, [ headingH3 ] );
} );

afterAll( () => {
Expand Down Expand Up @@ -98,19 +95,17 @@ describe( 'DocumentOutline', () => {

describe( 'header blocks present', () => {
it( 'should match snapshot', () => {
const blocks = [ headingParent, headingChild ].map(
( block, index ) => {
// Set client IDs to a predictable value.
return { ...block, clientId: `clientId_${ index }` };
}
);
const blocks = [ headingH2, headingH3 ].map( ( block, index ) => {
// Set client IDs to a predictable value.
return { ...block, clientId: `clientId_${ index }` };
} );
const wrapper = shallow( <DocumentOutline blocks={ blocks } /> );

expect( wrapper ).toMatchSnapshot();
} );

it( 'should render an item when only one heading provided', () => {
const blocks = [ headingParent ];
const blocks = [ headingH2 ];
const wrapper = shallow( <DocumentOutline blocks={ blocks } /> );

expect( wrapper.find( 'TableOfContentsItem' ) ).toHaveLength( 1 );
Expand All @@ -119,9 +114,9 @@ describe( 'DocumentOutline', () => {
it( 'should render two items when two headings and some paragraphs provided', () => {
const blocks = [
paragraph,
headingParent,
headingH2,
paragraph,
headingChild,
headingH3,
paragraph,
];
const wrapper = shallow( <DocumentOutline blocks={ blocks } /> );
Expand Down Expand Up @@ -149,16 +144,16 @@ describe( 'DocumentOutline', () => {
const outlineItemContentSelector =
'.document-outline__item-content';

const blocks = [ headingParent, nestedHeading ];
const blocks = [ headingH2, nestedHeading ];
const wrapper = mount( <DocumentOutline blocks={ blocks } /> );

//heading parent and nested heading should appear as items
// Unnested heading and nested heading should appear as items
const tableOfContentItems = wrapper.find(
tableOfContentItemsSelector
);
expect( tableOfContentItems ).toHaveLength( 2 );

//heading parent test
// Unnested heading test.
const firstItemLevels = tableOfContentItems
.at( 0 )
.find( outlineLevelsSelector );
Expand All @@ -169,21 +164,20 @@ describe( 'DocumentOutline', () => {
.at( 0 )
.find( outlineItemContentSelector )
.text()
).toEqual( 'Heading parent' );
).toEqual( 'Heading 2' );

//nested heading test
// Nested heading test.
const secondItemLevels = tableOfContentItems
.at( 1 )
.find( outlineLevelsSelector );
expect( secondItemLevels ).toHaveLength( 2 );
expect( secondItemLevels.at( 0 ).text() ).toEqual( 'Block Title' );
expect( secondItemLevels.at( 1 ).text() ).toEqual( 'H3' );
expect( secondItemLevels ).toHaveLength( 1 );
expect( secondItemLevels.at( 0 ).text() ).toEqual( 'H3' );
expect(
tableOfContentItems
.at( 1 )
.find( outlineItemContentSelector )
.text()
).toEqual( 'Heading child' );
).toEqual( 'Heading 3' );
} );
} );
} );