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

Fix max upload size error message #4203

Merged
merged 1 commit into from
May 22, 2018
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
102 changes: 102 additions & 0 deletions components/higher-order/with-notices/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/**
* External dependencies
*/
import uuid from 'uuid/v4';

/**
* WordPress dependencies
*/
import { Component, createHigherOrderComponent } from '@wordpress/element';

/**
* Internal dependencies
*/
import NoticeList from '../../notice/list';

/**
* Override the default edit UI to include notices if supported.
*
* @param {function|Component} OriginalComponent Original component.
* @return {Component} Wrapped component.
*/
export default createHigherOrderComponent( ( OriginalComponent ) => {
return class WrappedBlockEdit extends Component {
constructor() {
super( ...arguments );

this.createNotice = this.createNotice.bind( this );
this.createErrorNotice = this.createErrorNotice.bind( this );
this.removeNotice = this.removeNotice.bind( this );
this.removeAllNotices = this.removeAllNotices.bind( this );

this.state = {
noticeList: [],
};

this.noticeOperations = {
createNotice: this.createNotice,
createErrorNotice: this.createErrorNotice,
removeAllNotices: this.removeAllNotices,
removeNotice: this.removeNotice,
};
}

/**
* Function passed down as a prop that adds a new notice.
*
* @param {Object} notice Notice to add.
*/
createNotice( notice ) {
const noticeToAdd = notice.id ? notice : { ...notice, id: uuid() };
this.setState( ( state ) => ( {
noticeList: [ ...state.noticeList, noticeToAdd ],
} ) );
}

/**
* Function passed as a prop that adds a new error notice.
*
* @param {string} msg Error message of the notice.
*/
createErrorNotice( msg ) {
this.createNotice( { status: 'error', content: msg } );
}

/**
* Removes a notice by id.
*
* @param {string} id Id of the notice to remove.
*/
removeNotice( id ) {
this.setState( ( state ) => ( {
noticeList: state.noticeList.filter( ( notice ) => notice.id !== id ),
} ) );
}

/**
* Removes all notices
*/
removeAllNotices() {
this.setState( {
noticeList: [],
} );
}

render() {
return (
<OriginalComponent
noticeList={ this.state.noticeList }
Copy link
Member

Choose a reason for hiding this comment

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

Does that still need to be passed?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not being used, but passing it allows a user of the HOC to create a different UI and not rely on the provided noticeUI.

noticeOperations={ this.noticeOperations }
noticeUI={
Copy link
Member

Choose a reason for hiding this comment

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

This seems nicer! Wondering if it could make it clear things up even more if noticeOperations are properties on noticeUI? Do you think that would be strange?

Copy link
Member Author

Choose a reason for hiding this comment

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

Related with #4203 (comment) although we pass a noticeUI I wanted it to be a totally optional prop and allow components to ignore it if they just want the notices without relying on the default UI we provide.

this.state.noticeList.length > 0 && <NoticeList
className="components-with-notices-ui"
notices={ this.state.noticeList }
onRemove={ this.removeNotice }
/>
}
{ ...this.props }
/>
);
}
};
} );
1 change: 1 addition & 0 deletions components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ export { default as withFocusOutside } from './higher-order/with-focus-outside';
export { default as withFocusReturn } from './higher-order/with-focus-return';
export { default as withGlobalEvents } from './higher-order/with-global-events';
export { default as withInstanceId } from './higher-order/with-instance-id';
export { default as withNotices } from './higher-order/with-notices';
export { default as withSafeTimeout } from './higher-order/with-safe-timeout';
export { default as withSpokenMessages } from './higher-order/with-spoken-messages';
export { default as withState } from './higher-order/with-state';
14 changes: 12 additions & 2 deletions components/notice/list.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,21 @@ import { noop, omit } from 'lodash';
*/
import Notice from './';

function NoticeList( { notices, onRemove = noop, children } ) {
/**
* Renders a list of notices.
*
* @param {Object} $0 Props passed to the component.
* @param {Array} $0.notices Array of notices to render.
* @param {Function} $0.onRemove Function called when a notice should be removed / dismissed.
* @param {Object} $0.className Name of the class used by the component.
* @param {Object} $0.children Array of childs to be rendered inside the notice list.
* @return {Object} The rendered notices list.
*/
function NoticeList( { notices, onRemove = noop, className = 'components-notice-list', children } ) {
const removeNotice = ( id ) => () => onRemove( id );

return (
<div className="components-notice-list">
<div className={ className }>
{ children }
{ [ ...notices ].reverse().map( ( notice ) => (
<Notice { ...omit( notice, 'content' ) } key={ notice.id } onRemove={ removeNotice( notice.id ) }>
Expand Down
9 changes: 8 additions & 1 deletion components/placeholder/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,18 @@ import { isString } from 'lodash';
import './style.scss';
import Dashicon from '../dashicon';

function Placeholder( { icon, children, label, instructions, className, ...additionalProps } ) {
/**
* Renders a placeholder. Normally used by blocks to render their empty state.
*
* @param {Object} props The component props.
* @return {Object} The rendered placeholder.
*/
function Placeholder( { icon, children, label, instructions, className, notices, ...additionalProps } ) {
const classes = classnames( 'components-placeholder', className );

return (
<div { ...additionalProps } className={ classes }>
{ notices }
<div className="components-placeholder__label">
{ isString( icon ) ? <Dashicon icon={ icon } /> : icon }
{ label }
Expand Down
23 changes: 15 additions & 8 deletions core-blocks/gallery/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
SelectControl,
ToggleControl,
Toolbar,
withNotices,
} from '@wordpress/components';
import {
BlockControls,
Expand Down Expand Up @@ -44,7 +45,7 @@ export function defaultColumnsNumber( attributes ) {
return Math.min( 3, attributes.images.length );
}

export default class GalleryEdit extends Component {
class GalleryEdit extends Component {
constructor() {
super( ...arguments );

Expand Down Expand Up @@ -135,16 +136,17 @@ export default class GalleryEdit extends Component {

addFiles( files ) {
const currentImages = this.props.attributes.images || [];
const { setAttributes } = this.props;
editorMediaUpload(
files,
( images ) => {
const { noticeOperations, setAttributes } = this.props;
editorMediaUpload( {
allowedType: 'image',
filesList: files,
onFileChange: ( images ) => {
setAttributes( {
images: currentImages.concat( images ),
} );
},
'image',
);
onError: noticeOperations.createErrorNotice,
} );
}

componentWillReceiveProps( nextProps ) {
Expand All @@ -158,7 +160,7 @@ export default class GalleryEdit extends Component {
}

render() {
const { attributes, isSelected, className } = this.props;
const { attributes, isSelected, className, noticeOperations, noticeUI } = this.props;
const { images, columns = defaultColumnsNumber( attributes ), align, imageCrop, linkTo } = attributes;

const dropZone = (
Expand Down Expand Up @@ -210,6 +212,8 @@ export default class GalleryEdit extends Component {
accept="image/*"
type="image"
multiple
notices={ noticeUI }
onError={ noticeOperations.createErrorNotice }
/>
</Fragment>
);
Expand Down Expand Up @@ -241,6 +245,7 @@ export default class GalleryEdit extends Component {
/>
</PanelBody>
</InspectorControls>
{ noticeUI }
<ul className={ `${ className } align${ align } columns-${ columns } ${ imageCrop ? 'is-cropped' : '' }` }>
{ dropZone }
{ images.map( ( img, index ) => (
Expand Down Expand Up @@ -276,3 +281,5 @@ export default class GalleryEdit extends Component {
);
}
}

export default withNotices( GalleryEdit );
10 changes: 5 additions & 5 deletions core-blocks/gallery/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ export const settings = {
},
transform( files, onChange ) {
const block = createBlock( 'core/gallery' );
editorMediaUpload(
files,
( images ) => onChange( block.uid, { images } ),
'image'
);
editorMediaUpload( {
filesList: files,
onFileChange: ( images ) => onChange( block.uid, { images } ),
allowedType: 'image',
} );
return block;
},
},
Expand Down
26 changes: 20 additions & 6 deletions core-blocks/image/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
TextControl,
TextareaControl,
Toolbar,
withNotices,
} from '@wordpress/components';
import { withSelect } from '@wordpress/data';
import {
Expand Down Expand Up @@ -78,13 +79,13 @@ class ImageEdit extends Component {
getBlobByURL( url )
.then(
( file ) =>
editorMediaUpload(
[ file ],
( [ image ] ) => {
editorMediaUpload( {
filesList: [ file ],
onFileChange: ( [ image ] ) => {
setAttributes( { ...image } );
},
'image'
)
allowedType: 'image',
} )
);
}
}
Expand All @@ -107,6 +108,15 @@ class ImageEdit extends Component {
}

onSelectImage( media ) {
if ( ! media ) {
this.props.setAttributes( {
url: undefined,
alt: undefined,
id: undefined,
caption: undefined,
} );
return;
}
this.props.setAttributes( {
...pick( media, [ 'alt', 'id', 'caption', 'url' ] ),
width: undefined,
Expand Down Expand Up @@ -168,7 +178,7 @@ class ImageEdit extends Component {
}

render() {
const { attributes, setAttributes, isLargeViewport, isSelected, className, maxWidth, toggleSelection } = this.props;
const { attributes, setAttributes, isLargeViewport, isSelected, className, maxWidth, noticeOperations, noticeUI, toggleSelection } = this.props;
const { url, alt, caption, align, id, href, width, height } = attributes;

const controls = (
Expand Down Expand Up @@ -211,6 +221,8 @@ class ImageEdit extends Component {
} }
className={ className }
onSelect={ this.onSelectImage }
notices={ noticeUI }
onError={ noticeOperations.createErrorNotice }
accept="image/*"
type="image"
/>
Expand Down Expand Up @@ -306,6 +318,7 @@ class ImageEdit extends Component {
return (
<Fragment>
{ controls }
{ noticeUI }
<figure className={ classes }>
<ImageSize src={ url } dirtynessTrigger={ align }>
{ ( sizes ) => {
Expand Down Expand Up @@ -406,4 +419,5 @@ export default compose( [
};
} ),
withViewportMatch( { isLargeViewport: 'medium' } ),
withNotices,
] )( ImageEdit );
20 changes: 20 additions & 0 deletions editor/components/block-list/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,26 @@

margin-bottom: $block-spacing;

/**
* Notices
*/
.components-placeholder .components-with-notices-ui {
margin: -10px 20px 12px 20px;
width: calc( 100% - 40px );
}
.components-with-notices-ui {
margin: 0 0 12px 0;
width: 100%;

.notice {
margin-left: 0px;
margin-right: 0px;

p {
font-size: $default-font-size;
}
}
}

/**
* Warnings
Expand Down
11 changes: 9 additions & 2 deletions editor/components/media-placeholder/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,14 @@ class MediaPlaceholder extends Component {
}

onFilesUpload( files ) {
const { onSelect, type, multiple } = this.props;
const { onSelect, type, multiple, onError } = this.props;
const setMedia = multiple ? onSelect : ( [ media ] ) => onSelect( media );
editorMediaUpload( files, setMedia, type );
editorMediaUpload( {
allowedType: type,
filesList: files,
onFileChange: setMedia,
onError,
} );
}

render() {
Expand All @@ -80,6 +85,7 @@ class MediaPlaceholder extends Component {
onSelectUrl,
onHTMLDrop = noop,
multiple = false,
notices,
} = this.props;

return (
Expand All @@ -88,6 +94,7 @@ class MediaPlaceholder extends Component {
label={ labels.title }
instructions={ sprintf( __( 'Drag %s, upload a new one or select a file from your library.' ), labels.name ) }
className={ classnames( 'editor-media-placeholder', className ) }
notices={ notices }
>
<DropZone
onFilesDrop={ this.onFilesUpload }
Expand Down
Loading