-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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.
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
Add/549 list block indent buttons #717
Changes from 3 commits
40472da
b19ee7f
2418990
d5c0d0b
5f2c3d0
390e99a
ba33927
8dfdd74
1d93fbc
4cc20bf
f60e11d
abb531f
17a5168
1104141
27e1c0f
7156082
f8cce5a
befe07b
528f088
05288e1
68ab9ee
8e36e05
9c5b029
9ed5cf4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,34 @@ import Editable from 'components/editable'; | |
|
||
const { children, prop } = hpq; | ||
|
||
function activeEditorExecCommand( command ) { | ||
return () => { | ||
const ed = tinymce.activeEditor; | ||
if ( ed ) { | ||
ed.execCommand( command ); | ||
} | ||
}; | ||
} | ||
|
||
function listIsActive( listType ) { | ||
return ( { nodeName = 'OL', internalListType } ) => { | ||
return listType === ( internalListType ? internalListType : nodeName ); | ||
}; | ||
} | ||
|
||
function listSetType( listType, editorCommand ) { | ||
return ( { internalListType }, setAttributes ) => { | ||
if ( internalListType ) { | ||
// only change list types, don't toggle off internal lists | ||
if ( internalListType !== listType ) { | ||
activeEditorExecCommand( editorCommand )(); | ||
} | ||
} else { | ||
setAttributes( { nodeName: listType } ); | ||
} | ||
}; | ||
} | ||
|
||
registerBlock( 'core/list', { | ||
title: wp.i18n.__( 'List' ), | ||
icon: 'editor-ul', | ||
|
@@ -21,22 +49,35 @@ registerBlock( 'core/list', { | |
{ | ||
icon: 'editor-ul', | ||
title: wp.i18n.__( 'Convert to unordered' ), | ||
isActive: ( { nodeName = 'OL' } ) => nodeName === 'UL', | ||
onClick( attributes, setAttributes ) { | ||
setAttributes( { nodeName: 'UL' } ); | ||
} | ||
isActive: listIsActive( 'UL' ), | ||
onClick: listSetType( 'UL', 'InsertUnorderedList' ) | ||
}, | ||
{ | ||
icon: 'editor-ol', | ||
title: wp.i18n.__( 'Convert to ordered' ), | ||
isActive: ( { nodeName = 'OL' } ) => nodeName === 'OL', | ||
onClick( attributes, setAttributes ) { | ||
setAttributes( { nodeName: 'OL' } ); | ||
} | ||
} | ||
isActive: listIsActive( 'OL' ), | ||
onClick: listSetType( 'OL', 'InsertOrderedList' ) | ||
}, | ||
{ | ||
icon: 'editor-outdent', | ||
title: wp.i18n.__( 'Outdent list item' ), | ||
isActive: () => false, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this required to be defined? If so, we should probably update the behavior on rendering controls to allow it to be omitted (defaulting to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The isActive is required as |
||
onClick: activeEditorExecCommand( 'Outdent' ) | ||
}, | ||
{ | ||
icon: 'editor-indent', | ||
title: wp.i18n.__( 'Indent list item' ), | ||
isActive: () => false, | ||
onClick: activeEditorExecCommand( 'Indent' ) | ||
}, | ||
], | ||
|
||
edit( { attributes, setAttributes, focus, setFocus } ) { | ||
const onNodeChange = ( { parents } ) => { | ||
const list = parents.find( ( node ) => node.nodeName === 'UL' || node.nodeName === 'OL' ); | ||
const internalListType = list ? list.nodeName : null; | ||
setAttributes( { internalListType } ); | ||
}; | ||
const { nodeName = 'OL', values = [] } = attributes; | ||
return ( | ||
<Editable | ||
|
@@ -47,6 +88,7 @@ registerBlock( 'core/list', { | |
value={ values } | ||
focus={ focus } | ||
onFocus={ setFocus } | ||
onNodeChange={ onNodeChange } | ||
showAlignments | ||
className="blocks-list" /> | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
.blocks-list { | ||
list-style-position: inside; | ||
.blocks-list, .blocks-list ul, .blocks-list ol { | ||
padding-left: 20px; | ||
margin-left: 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'd want to avoid using globals like this. We can't assume TinyMCE is even used for the
Editable
component here.Unfortunately, it's very hard to find an alternative here. Maybe the "indent controls" should be considered as "inline toolbars" (the same way we do for formatting and alignment controls)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with that is we're moving code that likely no-one else is going to use into a common object purely to work around access issues. That doesn't appeal to me greatly either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I managed to avoid using the global but it required passing the editor via a callback.