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.

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

Merged
merged 7 commits into from
Oct 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 24 additions & 0 deletions editor/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,30 @@ export function stopTyping() {
};
}

/**
* Returns an action object used in signalling that the user toggled the sidebar
*
* @return {Object} Action object
*/
export function toggleSidebar() {
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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.

return {
type: 'TOGGLE_SIDEBAR',
};
}

/**
* Returns an action object used in signalling that the user switched the active sidebar tab panel
*
* @param {String} panel The panel name
* @return {Object} Action object
*/
export function setActivePanel( panel ) {
return {
type: 'SET_ACTIVE_PANEL',
panel,
};
}

/**
* Returns an action object used in signalling that the user toggled a sidebar panel
*
Expand Down
2 changes: 1 addition & 1 deletion editor/block-mover/style.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.editor-block-mover {
position: absolute;
top: 0;
left: 0;
left: 4px;
height: $text-editor-font-size * 4; // same height as an empty paragraph
padding: 6px 14px 6px 0; // handles hover area
z-index: z-index( '.editor-block-mover' );
Expand Down
63 changes: 63 additions & 0 deletions editor/block-settings-menu/content.js
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 );
82 changes: 39 additions & 43 deletions editor/block-settings-menu/index.js
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 );
24 changes: 19 additions & 5 deletions editor/block-settings-menu/style.scss
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);
}
}

Expand All @@ -35,7 +49,7 @@
display: block;
}

&:first-child {
margin-bottom: 4px;
.editor-block-settings-menu__content &:first-child {
Copy link
Member

Choose a reason for hiding this comment

The 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 .editor-block-settings-menu__content there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't you think it makes more sense to keep it in .editor-block-settings-menu__control because it's applied to this class when it's first on the list? I can see arguments for both which makes the choice difficult.

Copy link
Member

@gziolo gziolo Oct 9, 2017

Choose a reason for hiding this comment

The 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 .editor-block-settings-menu__inspector just to change the margin is most likely be too much here.

margin-bottom: 8px;
}
}
13 changes: 5 additions & 8 deletions editor/header/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -29,7 +29,7 @@ function Header( {
redo,
hasRedo,
hasUndo,
toggleSidebar,
onToggleSidebar,
isSidebarOpened,
} ) {
const count = multiSelectedBlockUids.length;
Expand Down Expand Up @@ -90,7 +90,7 @@ function Header( {
<PublishButton />
<IconButton
icon="admin-generic"
onClick={ toggleSidebar }
onClick={ onToggleSidebar }
isToggled={ isSidebarOpened }
label={ __( 'Settings' ) }
/>
Expand All @@ -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() ),
Copy link
Member

Choose a reason for hiding this comment

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

New action creator 👍

} )
)( Header );
14 changes: 5 additions & 9 deletions editor/sidebar/header.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ import { IconButton } from '@wordpress/components';
* Internal Dependencies
*/
import { getActivePanel } from '../selectors';
import { toggleSidebar, setActivePanel } from '../actions';

const SidebarHeader = ( { panel, onSetPanel, toggleSidebar } ) => {
const SidebarHeader = ( { panel, onSetPanel, onToggleSidebar } ) => {
return (
<div className="components-panel__header editor-sidebar__panel-tabs">
<button
Expand All @@ -32,7 +33,7 @@ const SidebarHeader = ( { panel, onSetPanel, toggleSidebar } ) => {
{ __( 'Block' ) }
</button>
<IconButton
onClick={ toggleSidebar }
onClick={ onToggleSidebar }
icon="no-alt"
label={ __( 'Close settings' ) }
/>
Expand All @@ -45,12 +46,7 @@ export default connect(
panel: getActivePanel( state ),
} ),
( dispatch ) => ( {
onSetPanel( panel ) {
dispatch( {
type: 'SET_ACTIVE_PANEL',
panel: panel,
} );
},
toggleSidebar: () => dispatch( { type: 'TOGGLE_SIDEBAR' } ),
onSetPanel: ( panel ) => dispatch( setActivePanel( panel ) ),
onToggleSidebar: () => dispatch( toggleSidebar() ),
} )
)( SidebarHeader );