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

Remove block switcher from header, add to multi block controls #7891

Merged
merged 6 commits into from
Jul 13, 2018
Merged
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
2 changes: 0 additions & 2 deletions edit-post/components/header/header-toolbar/index.js
Original file line number Diff line number Diff line change
@@ -15,7 +15,6 @@ import {
TableOfContents,
EditorHistoryRedo,
EditorHistoryUndo,
MultiBlocksSwitcher,
NavigableToolbar,
} from '@wordpress/editor';

@@ -34,7 +33,6 @@ function HeaderToolbar( { hasFixedToolbar, isLargeViewport } ) {
<EditorHistoryUndo />
<EditorHistoryRedo />
<TableOfContents />
<MultiBlocksSwitcher />
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this MultiBlocksSwitcher is already handling showing the switcher only for multi selection.

We should:

  • Delete this component since you duplicated its logic in block-toolbar
  • Or Use it in block-toolbar directly

{ hasFixedToolbar && isLargeViewport && (
<div className="edit-post-header-toolbar__block-toolbar">
<BlockToolbar />
2 changes: 1 addition & 1 deletion editor/components/block-list/block.js
Original file line number Diff line number Diff line change
@@ -407,7 +407,7 @@ export class BlockListBlock extends Component {
const shouldRenderMovers = ( isSelected || hoverArea === 'left' ) && ! showEmptyBlockSideInserter && ! isMultiSelecting && ! isPartOfMultiSelection && ! isTypingWithinBlock;
const shouldRenderBlockSettings = ( isSelected || hoverArea === 'right' ) && ! isMultiSelecting && ! isPartOfMultiSelection && ! isTypingWithinBlock;
const shouldShowBreadcrumb = isHovered && ! isEmptyDefaultBlock;
const shouldShowContextualToolbar = ! showSideInserter && isSelected && ! isTypingWithinBlock && isValid && ( ! hasFixedToolbar || ! isLargeViewport );
const shouldShowContextualToolbar = ! showSideInserter && ( ( isSelected && ! isTypingWithinBlock && isValid ) || isFirstMultiSelected ) && ( ! hasFixedToolbar || ! isLargeViewport );
const shouldShowMobileToolbar = shouldAppearSelected;
const { error, dragging } = this.state;

4 changes: 2 additions & 2 deletions editor/components/block-switcher/index.js
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@ import { castArray, get, some } from 'lodash';
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { __, _n, sprintf } from '@wordpress/i18n';
import { Dropdown, IconButton, Toolbar, PanelBody } from '@wordpress/components';
import { getBlockType, getPossibleBlockTransformations, switchToBlockType, hasChildBlocks } from '@wordpress/blocks';
import { compose, Component, Fragment } from '@wordpress/element';
@@ -65,7 +65,7 @@ export class BlockSwitcher extends Component {
onToggle();
}
};
const label = __( 'Change block type' );
const label = sprintf( _n( 'Change block type', 'Change type of %d blocks', blocks.length ), blocks.length );

return (
<Toolbar>
21 changes: 16 additions & 5 deletions editor/components/block-toolbar/index.js
Original file line number Diff line number Diff line change
@@ -8,29 +8,40 @@ import { withSelect } from '@wordpress/data';
*/
import './style.scss';
import BlockSwitcher from '../block-switcher';
import MultiBlocksSwitcher from '../block-switcher/multi-blocks-switcher';
import BlockControls from '../block-controls';
import BlockFormatControls from '../block-format-controls';

function BlockToolbar( { block, mode } ) {
if ( ! block || ! block.isValid || mode !== 'visual' ) {
function BlockToolbar( { blockUIDs, isValid, mode } ) {
if ( blockUIDs.length > 1 ) {
return (
<div className="editor-block-toolbar">
<MultiBlocksSwitcher />
</div>
);
}

if ( ! isValid || 'visual' !== mode ) {
return null;
}

return (
<div className="editor-block-toolbar">
<BlockSwitcher uids={ [ block.uid ] } />
<BlockSwitcher uids={ blockUIDs } />
<BlockControls.Slot />
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it makes sense to keep the slots in case we want to show multi-selection controls or something like that in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we do, and I'm working on that next week in another PR :) So I'll be rebasing to use these changes in that PR

<BlockFormatControls.Slot />
</div>
);
}

export default withSelect( ( select ) => {
const { getSelectedBlock, getBlockMode } = select( 'core/editor' );
const { getSelectedBlock, getBlockMode, getMultiSelectedBlockUids } = select( 'core/editor' );
const block = getSelectedBlock();
const blockUIDs = block ? [ block.uid ] : getMultiSelectedBlockUids();

return {
block,
blockUIDs,
isValid: block ? block.isValid : null,
mode: block ? getBlockMode( block.uid ) : null,
};
} )( BlockToolbar );