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.

Sign up for GitHub

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

Jump to bottom

Blocks: Improve UI for all Inspector Controls #5995

Merged
merged 2 commits into from
Apr 5, 2018
Merged

Blocks: Improve UI for all Inspector Controls #5995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dcad1f0
Blcoks: Improve UI for all Inspector Controls
gziolo Apr 5, 2018
9a61a2a
Blocks: Fix custom size slider for Paragraph block
gziolo Apr 5, 2018
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
11 changes: 10 additions & 1 deletion blocks/library/button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,14 @@
*/
import { __ } from '@wordpress/i18n';
import { Component } from '@wordpress/element';
import { Dashicon, IconButton, PanelColor, ToggleControl, withFallbackStyles } from '@wordpress/components';
import {
Dashicon,
IconButton,
PanelBody,
PanelColor,
ToggleControl,
withFallbackStyles,
} from '@wordpress/components';

/**
* Internal dependencies
Expand Down Expand Up @@ -96,6 +103,7 @@ class ButtonBlock extends Component {
/>
{ isSelected &&
<InspectorControls key="inspector">
<PanelBody>
<ToggleControl
label={ __( 'Wrap text' ) }
checked={ !! clear }
Expand All @@ -119,6 +127,7 @@ class ButtonBlock extends Component {
backgroundColor={ color }
isLargeText={ true }
/> }
</PanelBody>
</InspectorControls>
}
</span>,
Expand Down
5 changes: 3 additions & 2 deletions blocks/library/categories/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { Component } from '@wordpress/element';
import { Placeholder, Spinner, ToggleControl } from '@wordpress/components';
import { PanelBody, Placeholder, Spinner, ToggleControl } from '@wordpress/components';
import { withSelect } from '@wordpress/data';
import { __ } from '@wordpress/i18n';
import { times, unescape } from 'lodash';
Expand Down Expand Up @@ -148,7 +148,7 @@ class CategoriesBlock extends Component {

const inspectorControls = focus && (
<InspectorControls key="inspector">
<h3>{ __( 'Categories Settings' ) }</h3>
<PanelBody title={ __( 'Categories Settings' ) }>
<ToggleControl
label={ __( 'Display as dropdown' ) }
checked={ displayAsDropdown }
Expand All @@ -164,6 +164,7 @@ class CategoriesBlock extends Component {
checked={ showHierarchy }
onChange={ this.toggleShowHierarchy }
/>
</PanelBody>
</InspectorControls>
);

Expand Down
4 changes: 3 additions & 1 deletion blocks/library/columns/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import memoize from 'memize';
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { RangeControl } from '@wordpress/components';
import { PanelBody, RangeControl } from '@wordpress/components';

/**
* Internal dependencies
Expand Down Expand Up @@ -84,6 +84,7 @@ export const settings = {
/>
</BlockControls>,
<InspectorControls key="inspector">
<PanelBody>
<RangeControl
label={ __( 'Columns' ) }
value={ columns }
Expand All @@ -95,6 +96,7 @@ export const settings = {
min={ 2 }
max={ 6 }
/>
</PanelBody>
</InspectorControls>,
] : [],
<div className={ classes } key="container">
Expand Down
3 changes: 2 additions & 1 deletion blocks/library/cover-image/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export const settings = {
</Toolbar>
</BlockControls>,
<InspectorControls key="inspector">
<h2>{ __( 'Cover Image Settings' ) }</h2>
<PanelBody title={ __( 'Cover Image Settings' ) }>
<ToggleControl
label={ __( 'Fixed Background' ) }
checked={ !! hasParallax }
Expand All @@ -161,6 +161,7 @@ export const settings = {
max={ 100 }
step={ 10 }
/>
</PanelBody>
<PanelBody title={ __( 'Text Alignment' ) }>
{ alignmentToolbar }
</PanelBody>
Expand Down
4 changes: 3 additions & 1 deletion blocks/library/gallery/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
IconButton,
DropZone,
FormFileUpload,
PanelBody,
RangeControl,
SelectControl,
ToggleControl,
Expand Down Expand Up @@ -206,7 +207,7 @@ class GalleryBlock extends Component {
controls,
isSelected && (
<InspectorControls key="inspector">
<h2>{ __( 'Gallery Settings' ) }</h2>
<PanelBody title={ __( 'Gallery Settings' ) }>
{ images.length > 1 && <RangeControl
label={ __( 'Columns' ) }
value={ columns }
Expand All @@ -225,6 +226,7 @@ class GalleryBlock extends Component {
onChange={ this.setLinkTo }
options={ linkOptions }
/>
</PanelBody>
</InspectorControls>
),
<ul key="gallery" className={ `${ className } align${ align } columns-${ columns } ${ imageCrop ? 'is-cropped' : '' }` }>
Expand Down
5 changes: 3 additions & 2 deletions blocks/library/heading/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import { __, sprintf } from '@wordpress/i18n';
import { concatChildren } from '@wordpress/element';
import { Toolbar } from '@wordpress/components';
import { PanelBody, Toolbar } from '@wordpress/components';

/**
* Internal dependencies
Expand Down Expand Up @@ -121,7 +121,7 @@ export const settings = {
),
isSelected && (
<InspectorControls key="inspector">
<h3>{ __( 'Heading Settings' ) }</h3>
<PanelBody title={ __( 'Heading Settings' ) }>
<p>{ __( 'Level' ) }</p>
<Toolbar
controls={
Expand All @@ -141,6 +141,7 @@ export const settings = {
setAttributes( { align: nextAlign } );
} }
/>
</PanelBody>
</InspectorControls>
),
<RichText
Expand Down
11 changes: 9 additions & 2 deletions blocks/library/image/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { Component, compose } from '@wordpress/element';
import { getBlobByURL, revokeBlobURL, viewPort } from '@wordpress/utils';
import {
IconButton,
PanelBody,
SelectControl,
TextControl,
Toolbar,
Expand Down Expand Up @@ -201,8 +202,13 @@ class ImageBlock extends Component {
controls,
isSelected && (
<InspectorControls key="inspector">
<h2>{ __( 'Image Settings' ) }</h2>
<TextControl label={ __( 'Textual Alternative' ) } value={ alt } onChange={ this.updateAlt } help={ __( 'Describe the purpose of the image. Leave empty if the image is not a key part of the content.' ) } />
<PanelBody title={ __( 'Image Settings' ) }>
<TextControl
label={ __( 'Textual Alternative' ) }
value={ alt }
onChange={ this.updateAlt }
help={ __( 'Describe the purpose of the image. Leave empty if the image is not a key part of the content.' ) }
/>
{ ! isEmpty( availableSizes ) && (
<SelectControl
label={ __( 'Size' ) }
Expand All @@ -214,6 +220,7 @@ class ImageBlock extends Component {
onChange={ this.updateImageURL }
/>
) }
</PanelBody>
</InspectorControls>
),
<figure key="image" className={ classes } style={ figureStyle }>
Expand Down
4 changes: 3 additions & 1 deletion blocks/library/latest-posts/block.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { stringify } from 'querystringify';
*/
import { Component } from '@wordpress/element';
import {
PanelBody,
Placeholder,
QueryControls,
RangeControl,
Expand Down Expand Up @@ -54,7 +55,7 @@ class LatestPostsBlock extends Component {

const inspectorControls = isSelected && (
<InspectorControls key="inspector">
<h3>{ __( 'Latest Posts Settings' ) }</h3>
<PanelBody title={ __( 'Latest Posts Settings' ) }>
<QueryControls
{ ...{ order, orderBy } }
numberOfItems={ postsToShow }
Expand All @@ -79,6 +80,7 @@ class LatestPostsBlock extends Component {
max={ ! hasPosts ? MAX_POSTS_COLUMNS : Math.min( MAX_POSTS_COLUMNS, latestPosts.length ) }
/>
}
</PanelBody>
</InspectorControls>
);

Expand Down
4 changes: 3 additions & 1 deletion blocks/library/more/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { compact } from 'lodash';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { ToggleControl } from '@wordpress/components';
import { PanelBody, ToggleControl } from '@wordpress/components';
import { Component, RawHTML } from '@wordpress/element';

/**
Expand Down Expand Up @@ -100,11 +100,13 @@ export const settings = {
return [
isSelected && (
<InspectorControls key="inspector">
<PanelBody>
<ToggleControl
label={ __( 'Hide the teaser before the "More" tag' ) }
checked={ !! noTeaser }
onChange={ toggleNoTeaser }
/>
</PanelBody>
</InspectorControls>
),
<div key="more-tag" className="wp-block-more">
Expand Down
4 changes: 3 additions & 1 deletion blocks/library/paragraph/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
justify-content: space-between;
}

.blocks-font-size .blocks-range-control__slider + .dashicon {
.blocks-paragraph__custom-size-slider {
.components-range-control__slider + .dashicon {
width: 30px;
height: 30px;
}
}
7 changes: 5 additions & 2 deletions blocks/library/paragraph/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ class ParagraphBlock extends Component {
if ( customFontSize ) {
return customFontSize;
}

return FONT_SIZES.regular;
Copy link
Member

@jorgefilipecosta jorgefilipecosta Apr 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

We are setting the default size for font Size as regular making the range control start in some value. But we don't apply the regular class.
This may cause problems, imagine for example I set a CSS rule for paragraphs nested inside the cover image to have size 20px. These changes make the default size for paragraph appear as 16px but we don't set the class so the size will be in fact the one set with CSS for paragraphs inside cover image 20px.
Essentially I think there is a difference of state between not having size specified, no class et all where we use the default css rules depending on the context, and having regular font size where we use the regular class.

Copy link
Member Author

@gziolo gziolo Apr 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This may cause problems, imagine for example I set a CSS rule for paragraphs nested inside the cover image the have size 20px.

Yes, I didn't think about that. Good catch. If you use fontSize or customFontSize prop it won't be an issue. Updating CSS is tricky. I got your point though.

Any idea how to move the slider to the proper position without updating the state then? :)

At the moment it's a very bad user experience when you start updating the font size and it gets much bigger than it initially was.

Copy link
Member

@jorgefilipecosta jorgefilipecosta Apr 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I agree the solution we had is not perfect, I think a possible improvement maybe use getComputedStyles to try to understand the actual size of font, and position the initial range slider position in that value, but without setting the value itself (we would have an initial position prop in the slider to set it to initial value when no value exists) the value would only be set if the user changed it. So we would still have a difference between the state user explictly set some value or no value was chosen.

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened PR to revert this change. This is more complex than I expected and not that important to spend a significant amount of time on it. See: #6075.

}

setFontSize( fontSizeValue ) {
Expand Down Expand Up @@ -186,8 +188,9 @@ class ParagraphBlock extends Component {
</Button>
</div>
<RangeControl
className="blocks-paragraph__custom-size-slider"
label={ __( 'Custom Size' ) }
value={ fontSize || '' }
value={ fontSize }
onChange={ ( value ) => this.setFontSize( value ) }
min={ 12 }
max={ 100 }
Expand Down Expand Up @@ -236,7 +239,7 @@ class ParagraphBlock extends Component {
style={ {
backgroundColor: backgroundColor,
color: textColor,
fontSize: fontSize ? fontSize + 'px' : undefined,
fontSize: fontSize !== FONT_SIZES.regular ? fontSize + 'px' : undefined,
textAlign: align,
} }
value={ content }
Expand Down
4 changes: 3 additions & 1 deletion blocks/library/text-columns/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { times } from 'lodash';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { RangeControl } from '@wordpress/components';
import { PanelBody, RangeControl } from '@wordpress/components';

/**
* Internal dependencies
Expand Down Expand Up @@ -73,13 +73,15 @@ export const settings = {
),
isSelected && (
<InspectorControls key="inspector">
<PanelBody>
<RangeControl
label={ __( 'Columns' ) }
value={ columns }
onChange={ ( value ) => setAttributes( { columns: value } ) }
min={ 2 }
max={ 4 }
/>
</PanelBody>
</InspectorControls>
),
<div className={ `${ className } align${ width } columns-${ columns }` } key="block">
Expand Down
12 changes: 9 additions & 3 deletions components/range-control/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* WordPress dependencies
*/
import classnames from 'classnames';
import { __ } from '@wordpress/i18n';

/**
Expand All @@ -10,13 +11,18 @@ import { BaseControl, Button, Dashicon } from '../';
import withInstanceId from '../higher-order/with-instance-id';
import './style.scss';

function RangeControl( { label, value, instanceId, onChange, beforeIcon, afterIcon, help, allowReset, ...props } ) {
function RangeControl( { className, label, value, instanceId, onChange, beforeIcon, afterIcon, help, allowReset, ...props } ) {
const id = `inspector-range-control-${ instanceId }`;
const onChangeValue = ( event ) => onChange( Number( event.target.value ) );

return (
<BaseControl label={ label } id={ id } help={ help } className="components-range-control">
{ beforeIcon && <Dashicon icon={ beforeIcon } size={ 20 } /> }
<BaseControl
label={ label }
id={ id }
help={ help }
className={ classnames( 'components-range-control', className ) }
>
{ beforeIcon && <Dashicon icon={ beforeIcon } /> }
<input
className="components-range-control__slider"
id={ id }
Expand Down
6 changes: 5 additions & 1 deletion edit-post/components/sidebar/block-inspector-panel/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,12 @@
color: $dark-gray-500;
}

&.editor-block-inspector__advanced {
&:first-child {
border-top: 1px solid $light-gray-500;
margin-top: 16px;
}

&:last-child {
margin-bottom: -16px;
}
}
2 changes: 0 additions & 2 deletions editor/components/block-inspector/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
.editor-block-inspector__card {
display: flex;
align-items: flex-start;
border-bottom: 1px solid $light-gray-500;
margin: -16px;
margin-bottom: 16px;
padding: 16px;
}

Expand Down