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

Components: Use a div wrapper for the toolbar #3001

Merged
merged 1 commit into from
Oct 13, 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
22 changes: 10 additions & 12 deletions blocks/library/audio/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,18 +94,16 @@ registerBlockType( 'core/audio', {
onChange={ updateAlignment }
/>
<Toolbar>
<li>
<Button
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit audio' ),
} }
type="audio"
onClick={ switchToEditing }
>
<Dashicon icon="edit" />
</Button>
</li>
<Button
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit audio' ),
} }
type="audio"
onClick={ switchToEditing }
>
<Dashicon icon="edit" />
</Button>
</Toolbar>
</BlockControls>
);
Expand Down
24 changes: 11 additions & 13 deletions blocks/library/cover-image/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,17 @@ registerBlockType( 'core/cover-image', {
/>

<Toolbar>
<li>
<MediaUploadButton
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit image' ),
} }
onSelect={ onSelectImage }
type="image"
value={ id }
>
<Dashicon icon="edit" />
</MediaUploadButton>
</li>
<MediaUploadButton
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit image' ),
} }
onSelect={ onSelectImage }
type="image"
value={ id }
>
<Dashicon icon="edit" />
</MediaUploadButton>
</Toolbar>
</BlockControls>,
<InspectorControls key="inspector">
Expand Down
28 changes: 13 additions & 15 deletions blocks/library/gallery/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,21 +123,19 @@ class GalleryBlock extends Component {
/>
{ !! images.length && (
<Toolbar>
<li>
<MediaUploadButton
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit Gallery' ),
} }
onSelect={ this.onSelectImages }
type="image"
multiple
gallery
value={ images.map( ( img ) => img.id ) }
>
<Dashicon icon="edit" />
</MediaUploadButton>
</li>
<MediaUploadButton
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit Gallery' ),
} }
onSelect={ this.onSelectImages }
type="image"
multiple
gallery
value={ images.map( ( img ) => img.id ) }
>
<Dashicon icon="edit" />
</MediaUploadButton>
</Toolbar>
) }
</BlockControls>
Expand Down
24 changes: 11 additions & 13 deletions blocks/library/image/block.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,17 @@ class ImageBlock extends Component {
/>

<Toolbar>
<li>
<MediaUploadButton
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit image' ),
} }
onSelect={ this.onSelectImage }
type="image"
value={ id }
>
<Dashicon icon="edit" />
</MediaUploadButton>
</li>
<MediaUploadButton
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit image' ),
} }
onSelect={ this.onSelectImage }
type="image"
value={ id }
>
<Dashicon icon="edit" />
</MediaUploadButton>
<UrlInputButton onChange={ this.onSetHref } url={ href } />
</Toolbar>
</BlockControls>
Expand Down
2 changes: 1 addition & 1 deletion blocks/library/table/editor.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.editor-visual-editor__block[data-type="core/table"] {

.editor-block-toolbar__group > div {
.editor-block-toolbar__group > div:not(.editor-block-toolbar__mobile-tools) {
display: flex;
}

Expand Down
20 changes: 9 additions & 11 deletions blocks/library/table/table-block.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,17 +113,15 @@ export default class TableBlock extends Component {
focus && (
<BlockControls key="menu">
<Toolbar>
<li>
<DropdownMenu
icon="editor-table"
label={ __( 'Edit Table' ) }
controls={
TABLE_CONTROLS.map( ( control ) => ( {
...control,
onClick: () => control.onClick( this.state.editor ),
} ) ) }
/>
</li>
<DropdownMenu
icon="editor-table"
label={ __( 'Edit Table' ) }
controls={
TABLE_CONTROLS.map( ( control ) => ( {
...control,
onClick: () => control.onClick( this.state.editor ),
} ) ) }
/>
</Toolbar>
</BlockControls>
),
Expand Down
24 changes: 11 additions & 13 deletions blocks/library/video/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,17 @@ registerBlockType( 'core/video', {
/>

<Toolbar>
<li>
<MediaUploadButton
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit video' ),
} }
onSelect={ onSelectVideo }
type="video"
value={ id }
>
<Dashicon icon="edit" />
</MediaUploadButton>
</li>
<MediaUploadButton
buttonProps={ {
className: 'components-icon-button components-toolbar__control',
'aria-label': __( 'Edit video' ),
} }
onSelect={ onSelectVideo }
type="video"
value={ id }
>
<Dashicon icon="edit" />
</MediaUploadButton>
</Toolbar>
</BlockControls>,

Expand Down
4 changes: 2 additions & 2 deletions blocks/url-input/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class UrlInputButton extends Component {
const { expanded } = this.state;

return (
<li className="blocks-url-input__button">
<div className="blocks-url-input__button">
Copy link
Member

@gziolo gziolo Oct 13, 2017

Choose a reason for hiding this comment

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

I double checked and it looks like it is only used in the toolbar at the moment. It makes a lot of sense to use div here as it makes it more reusable 👍

<IconButton
icon="admin-links"
label={ __( 'Edit Link' ) }
Expand All @@ -67,7 +67,7 @@ class UrlInputButton extends Component {
/>
</form>
}
</li>
</div>
);
}
}
Expand Down
2 changes: 1 addition & 1 deletion blocks/url-input/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
}

// Toolbar button
ul.components-toolbar > li.blocks-url-input__button {
.components-toolbar > .blocks-url-input__button {
position: inherit; // let the dialog position according to parent
}

Expand Down
8 changes: 4 additions & 4 deletions components/toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ function Toolbar( { controls = [], children, className } ) {
}

return (
<ul className={ classnames( 'components-toolbar', className ) }>
<div className={ classnames( 'components-toolbar', className ) }>
{ controlSets.reduce( ( result, controlSet, setIndex ) => [
...result,
...controlSet.map( ( control, controlIndex ) => (
<li
<div
key={ [ setIndex, controlIndex ].join() }
className={ setIndex > 0 && controlIndex === 0 ? 'has-left-divider' : null }
>
Expand All @@ -47,11 +47,11 @@ function Toolbar( { controls = [], children, className } ) {
disabled={ control.isDisabled }
/>
{ control.children }
</li>
</div>
) ),
], [] ) }
{ children }
</ul>
</div>
);
}

Expand Down
8 changes: 3 additions & 5 deletions components/toolbar/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@
display: inline-flex;
}

ul.components-toolbar {
list-style-type: none;

&> li {
div.components-toolbar {
&> div {
display: inline-flex;
margin: 0;
}

&> li + li {
&> div + div {
margin-left: -3px;

&.has-left-divider {
Expand Down
1 change: 0 additions & 1 deletion components/toolbar/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ describe( 'Toolbar', () => {
];
const toolbar = shallow( <Toolbar controls={ controls } /> );
const listItem = toolbar.find( 'IconButton' );
Copy link
Member

Choose a reason for hiding this comment

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

I found this jest-enzyme yesterday. It might simplify testing components. We can also experiment with snaphsots to minimize the number of assertions where we validate HTML, which should be an implementation detail that is more likely to change.

expect( toolbar.type() ).toBe( 'ul' );
expect( listItem.props() ).toMatchObject( {
icon: 'wordpress',
label: 'WordPress',
Expand Down
4 changes: 2 additions & 2 deletions editor/sidebar/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@
margin: 1.5em 0;
}

ul.components-toolbar {
div.components-toolbar {
box-shadow: none;
margin-bottom: 1.5em;
}

p + ul.components-toolbar {
p + div.components-toolbar {
margin-top: -1em;
}
}
Expand Down