-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Blocks: Introduce block settings menu toggle #2884
Changes from all commits
1e6e9b1
269c57c
df203f4
3a3908a
2456fd2
c3b8cd1
8977595
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 |
---|---|---|
@@ -0,0 +1,63 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { connect } from 'react-redux'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { IconButton } from '@wordpress/components'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { isEditorSidebarOpened } from '../selectors'; | ||
import { selectBlock, removeBlock, toggleSidebar, setActivePanel } from '../actions'; | ||
|
||
function BlockSettingsMenuContent( { onDelete, onSelect, isSidebarOpened, onToggleSidebar, onShowInspector } ) { | ||
const toggleInspector = () => { | ||
onSelect(); | ||
onShowInspector(); | ||
if ( ! isSidebarOpened ) { | ||
onToggleSidebar(); | ||
} | ||
}; | ||
|
||
return ( | ||
<div className="editor-block-settings-menu__content"> | ||
<IconButton | ||
className="editor-block-settings-menu__control" | ||
onClick={ toggleInspector } | ||
icon="admin-generic" | ||
label={ __( 'Show inspector' ) } | ||
/> | ||
<IconButton | ||
className="editor-block-settings-menu__control" | ||
onClick={ onDelete } | ||
icon="trash" | ||
label={ __( 'Delete the block' ) } | ||
/> | ||
</div> | ||
); | ||
} | ||
|
||
export default connect( | ||
( state ) => ( { | ||
isSidebarOpened: isEditorSidebarOpened( state ), | ||
} ), | ||
( dispatch, ownProps ) => ( { | ||
onDelete() { | ||
dispatch( removeBlock( ownProps.uid ) ); | ||
}, | ||
onSelect() { | ||
dispatch( selectBlock( ownProps.uid ) ); | ||
}, | ||
onShowInspector() { | ||
dispatch( setActivePanel( 'block' ) ); | ||
}, | ||
onToggleSidebar() { | ||
dispatch( toggleSidebar() ); | ||
}, | ||
} ) | ||
)( BlockSettingsMenuContent ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,70 +1,66 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import classnames from 'classnames'; | ||
import { connect } from 'react-redux'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { IconButton } from '@wordpress/components'; | ||
import { Component } from '@wordpress/element'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import './style.scss'; | ||
import { isEditorSidebarOpened } from '../selectors'; | ||
import BlockSettingsMenuContent from './content'; | ||
import { selectBlock } from '../actions'; | ||
|
||
function BlockSettingsMenu( { onDelete, onSelect, isSidebarOpened, toggleSidebar, setActivePanel } ) { | ||
const toggleInspector = () => { | ||
onSelect(); | ||
setActivePanel(); | ||
if ( ! isSidebarOpened ) { | ||
toggleSidebar(); | ||
} | ||
}; | ||
class BlockSettingsMenu extends Component { | ||
constructor() { | ||
super( ...arguments ); | ||
this.toggleMenu = this.toggleMenu.bind( this ); | ||
this.state = { | ||
opened: false, | ||
}; | ||
} | ||
|
||
return ( | ||
<div className="editor-block-settings-menu"> | ||
<IconButton | ||
className="editor-block-settings-menu__control" | ||
onClick={ toggleInspector } | ||
icon="admin-generic" | ||
label={ __( 'Show inspector' ) } | ||
/> | ||
<IconButton | ||
className="editor-block-settings-menu__control" | ||
onClick={ onDelete } | ||
icon="trash" | ||
label={ __( 'Delete the block' ) } | ||
/> | ||
</div> | ||
); | ||
toggleMenu() { | ||
this.props.onSelect(); | ||
this.setState( ( state ) => ( { | ||
opened: ! state.opened, | ||
} ) ); | ||
} | ||
|
||
render() { | ||
const { opened } = this.state; | ||
const { uid } = this.props; | ||
const toggleClassname = classnames( 'editor-block-settings-menu__toggle', 'editor-block-settings-menu__control', { | ||
'is-opened': opened, | ||
} ); | ||
|
||
return ( | ||
<div className="editor-block-settings-menu"> | ||
<IconButton | ||
className={ toggleClassname } | ||
onClick={ this.toggleMenu } | ||
icon="ellipsis" | ||
label={ opened ? __( 'Close Settings Menu' ) : __( 'Open Settings Menu' ) } | ||
/> | ||
|
||
{ opened && <BlockSettingsMenuContent uid={ uid } /> } | ||
</div> | ||
); | ||
} | ||
} | ||
|
||
export default connect( | ||
( state ) => ( { | ||
isSidebarOpened: isEditorSidebarOpened( state ), | ||
} ), | ||
undefined, | ||
( dispatch, ownProps ) => ( { | ||
onDelete() { | ||
dispatch( { | ||
type: 'REMOVE_BLOCKS', | ||
uids: [ ownProps.uid ], | ||
} ); | ||
}, | ||
onSelect() { | ||
dispatch( selectBlock( ownProps.uid ) ); | ||
}, | ||
setActivePanel() { | ||
dispatch( { | ||
type: 'SET_ACTIVE_PANEL', | ||
panel: 'block', | ||
} ); | ||
}, | ||
toggleSidebar() { | ||
dispatch( { type: 'TOGGLE_SIDEBAR' } ); | ||
}, | ||
} ) | ||
)( BlockSettingsMenu ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,29 @@ | ||
.editor-block-settings-menu { | ||
position: absolute; | ||
top: 0; | ||
right: 0; | ||
height: $text-editor-font-size * 4; // same height as an empty paragraph | ||
right: 4px; | ||
padding: 6px 0 6px 14px; // handles hover area | ||
|
||
// Mobile | ||
display: none; | ||
|
||
@include break-small { | ||
display: block; | ||
display: flex; | ||
flex-direction: column; | ||
} | ||
} | ||
|
||
.editor-block-settings-menu__content { | ||
margin-top: 8px; | ||
} | ||
|
||
.editor-block-settings-menu__toggle { | ||
transform: rotate( 90deg ); | ||
transition-duration: 0.3s; | ||
transition-property: transform; | ||
|
||
&.is-opened { | ||
transform: rotate(0); | ||
} | ||
} | ||
|
||
|
@@ -35,7 +49,7 @@ | |
display: block; | ||
} | ||
|
||
&:first-child { | ||
margin-bottom: 4px; | ||
.editor-block-settings-menu__content &:first-child { | ||
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. It looks like this can be moved to line 17. We have rules for 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. Don't you think it makes more sense to keep it in 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. It takes some time to process those class names either way. CSS is complicated. We could add another class name, but having a section |
||
margin-bottom: 8px; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,7 @@ import PreviewButton from './preview-button'; | |
import ModeSwitcher from './mode-switcher'; | ||
import Inserter from '../inserter'; | ||
import { getMultiSelectedBlockUids, hasEditorUndo, hasEditorRedo, isEditorSidebarOpened } from '../selectors'; | ||
import { clearSelectedBlock } from '../actions'; | ||
import { clearSelectedBlock, toggleSidebar, removeBlocks } from '../actions'; | ||
|
||
function Header( { | ||
multiSelectedBlockUids, | ||
|
@@ -29,7 +29,7 @@ function Header( { | |
redo, | ||
hasRedo, | ||
hasUndo, | ||
toggleSidebar, | ||
onToggleSidebar, | ||
isSidebarOpened, | ||
} ) { | ||
const count = multiSelectedBlockUids.length; | ||
|
@@ -90,7 +90,7 @@ function Header( { | |
<PublishButton /> | ||
<IconButton | ||
icon="admin-generic" | ||
onClick={ toggleSidebar } | ||
onClick={ onToggleSidebar } | ||
isToggled={ isSidebarOpened } | ||
label={ __( 'Settings' ) } | ||
/> | ||
|
@@ -109,12 +109,9 @@ export default connect( | |
} ), | ||
( dispatch ) => ( { | ||
onDeselect: () => dispatch( clearSelectedBlock() ), | ||
onRemove: ( uids ) => dispatch( { | ||
type: 'REMOVE_BLOCKS', | ||
uids, | ||
} ), | ||
onRemove: ( uids ) => dispatch( removeBlocks( uids ) ), | ||
undo: () => dispatch( { type: 'UNDO' } ), | ||
redo: () => dispatch( { type: 'REDO' } ), | ||
toggleSidebar: () => dispatch( { type: 'TOGGLE_SIDEBAR' } ), | ||
onToggleSidebar: () => dispatch( toggleSidebar() ), | ||
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. New action creator 👍 |
||
} ) | ||
)( Header ); |
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 noticed that we have unit tests for a few existing action creators, should we cover those 2, too?
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.
Yeah! I know and I've written some of these, but honestly, I'm not sure these add any value for action creators just returning an action with the arguments as properties of the object.
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.
Yes, sometimes it's hard to justify the need for writing tests, but at the same time it's very easy to write them. It's your call, as I mentioned already. It's not a blocker.