Skip to content

Display heading levels dynamically in Heading Options when Heading block is selected #14689

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

Closed
igmoweb wants to merge 7 commits into from
Closed

Display heading levels dynamically in Heading Options when Heading block is selected #14689

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6791ea6
Dynamically display heading options when Heading Block is selected.
igmoweb Mar 28, 2019
a75eef2
Allow to pass a range to HeadingToolbar component
igmoweb Mar 28, 2019
590592c
Added comment to Heading Edit component
igmoweb Mar 28, 2019
d0ab84a
Simplify Heading Block code with level enum property
igmoweb Mar 29, 2019
e5d5256
Set camel-cased variable
igmoweb Apr 3, 2019
cf484c4
Performance improvements in Heading block
igmoweb Apr 3, 2019
9e71339
Set camel-cased variable
igmoweb Apr 3, 2019
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
Prev Previous commit
Next Next commit
Added comment to Heading Edit component
  • Loading branch information
@igmoweb
igmoweb committed Mar 28, 2019
commit 590592cddb160e2336ba20af960536955a511580
3 changes: 2 additions & 1 deletion packages/block-library/src/heading/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function HeadingEdit( {
const tagName = 'h' + level;
const BlockControlsLevelsRange = ( levelChoices.length <= 3 ) ?
Copy link
Member

@aduth aduth Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be camel-cased with an initial lower-case letter.

https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript/#naming-conventions

Copy link
Author

@igmoweb igmoweb Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

levelChoices :
levelChoices.slice( 1, 4 );
levelChoices.slice( 1, 4 ); //
Copy link
Member

@aduth aduth Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I'm following what the logic is here as far as the second, third, fourth, and fifth level choice options being significant. I sense you'd considered to add an inline comment but forgot to complete it?

Taking a guess with the default options, is it mostly about limiting to a select few common options, and omitting H1 specifically? In that case it seems more accurate to express as something like take( without( levelChoices, [ 1 ] ), 3 ) (Array#slice can work just as well)

https://lodash.com/docs/4.17.11#take
https://lodash.com/docs/4.17.11#without

This also seems like it would help avoid having to test length <= 3 and just have a single consistent expression for computing the level choices.

Copy link
Author

@igmoweb igmoweb Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduth What I'm trying to do here is to replicate the previous behavior in the BlockControls options (not the sidebar). Right now, it displays only 3 headings: H2, H3 and H4. So if the levels list contains 3 or less options, it will display all of them, otherwise, slice the array and display 3 elements starting by the second one. I think using Lodash for that would be an overkill for such a simple operation.

return (
<Fragment>
<BlockControls>
Expand Down Expand Up @@ -83,6 +83,7 @@ function HeadingEdit( {
}

export default withSelect( ( select ) => {
// Parse the h1,h2,h3... choices to level numbers and pass it as a prop.
const levelChoices = get(
select( 'core/blocks' ).getBlockType( 'core/heading' ),
[ 'attributes', 'content', 'selector' ],
Expand Down