-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Update/edit flow for blocks which have a media placeholder #14918
Changes from all commits
13146a7
c7c606e
5646a72
cab27f9
e3c3866
a659801
e9cd606
2723aa8
0e7129f
18fdced
d7c3b39
5e23de5
84efe80
7948ece
2b129f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,8 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import classnames from 'classnames'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
|
@@ -6,7 +11,10 @@ import { | |
Disabled, | ||
IconButton, | ||
PanelBody, | ||
Path, | ||
Rect, | ||
SelectControl, | ||
SVG, | ||
ToggleControl, | ||
Toolbar, | ||
withNotices, | ||
|
@@ -26,10 +34,6 @@ import { __ } from '@wordpress/i18n'; | |
* Internal dependencies | ||
*/ | ||
import icon from './icon'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { createUpgradedEmbedBlock } from '../embed/util'; | ||
|
||
const ALLOWED_MEDIA_TYPES = [ 'audio' ]; | ||
|
@@ -103,7 +107,7 @@ class AudioEdit extends Component { | |
const { setAttributes, isSelected, className, noticeOperations, noticeUI } = this.props; | ||
const { editing } = this.state; | ||
const switchToEditing = () => { | ||
this.setState( { editing: true } ); | ||
this.setState( { editing: ! this.state.editing } ); | ||
}; | ||
const onSelectAudio = ( media ) => { | ||
if ( ! media || ! media.url ) { | ||
|
@@ -118,22 +122,35 @@ class AudioEdit extends Component { | |
setAttributes( { src: media.url, id: media.id } ); | ||
this.setState( { src: media.url, editing: false } ); | ||
}; | ||
const editImageIcon = ( <SVG width={ 20 } height={ 20 } viewBox="0 0 20 20"><Rect x={ 11 } y={ 3 } width={ 7 } height={ 5 } rx={ 1 } /><Rect x={ 2 } y={ 12 } width={ 7 } height={ 5 } rx={ 1 } /><Path d="M13,12h1a3,3,0,0,1-3,3v2a5,5,0,0,0,5-5h1L15,9Z" /><Path d="M4,8H3l2,3L7,8H6A3,3,0,0,1,9,5V3A5,5,0,0,0,4,8Z" /></SVG> ); | ||
if ( editing ) { | ||
return ( | ||
<MediaPlaceholder | ||
icon={ <BlockIcon icon={ icon } /> } | ||
className={ className } | ||
onSelect={ onSelectAudio } | ||
onSelectURL={ this.onSelectURL } | ||
accept="audio/*" | ||
allowedTypes={ ALLOWED_MEDIA_TYPES } | ||
value={ this.props.attributes } | ||
notices={ noticeUI } | ||
onError={ noticeOperations.createErrorNotice } | ||
/> | ||
<Fragment> | ||
<BlockControls> | ||
{ !! src && ( <Toolbar> | ||
<IconButton | ||
className={ classnames( 'components-icon-button components-toolbar__control', { 'is-active': this.state.editing } ) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to add the components-icon-button classname, this is added by the IconButton component itself. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, it seems like |
||
label={ __( 'Edit audio' ) } | ||
onClick={ switchToEditing } | ||
icon={ editImageIcon } | ||
/> | ||
</Toolbar> ) } | ||
</BlockControls> | ||
<MediaPlaceholder | ||
icon={ <BlockIcon icon={ icon } /> } | ||
className={ className } | ||
onCancel={ !! src && switchToEditing } | ||
onSelect={ onSelectAudio } | ||
onSelectURL={ this.onSelectURL } | ||
accept="audio/*" | ||
allowedTypes={ ALLOWED_MEDIA_TYPES } | ||
value={ this.props.attributes } | ||
notices={ noticeUI } | ||
onError={ noticeOperations.createErrorNotice } | ||
/> | ||
</Fragment> | ||
); | ||
} | ||
|
||
/* eslint-disable jsx-a11y/no-static-element-interactions, jsx-a11y/onclick-has-role, jsx-a11y/click-events-have-key-events */ | ||
return ( | ||
<Fragment> | ||
|
@@ -143,7 +160,7 @@ class AudioEdit extends Component { | |
className="components-icon-button components-toolbar__control" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same issue with the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems a shame to have to duplicate the code for this IconButton. Maybe it can be extracted to a separate component in the same file? Or the rendering logic could be reworked. |
||
label={ __( 'Edit audio' ) } | ||
onClick={ switchToEditing } | ||
icon="edit" | ||
icon={ editImageIcon } | ||
/> | ||
</Toolbar> | ||
</BlockControls> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,10 @@ import { | |
FocalPointPicker, | ||
IconButton, | ||
PanelBody, | ||
Path, | ||
RangeControl, | ||
Rect, | ||
SVG, | ||
ToggleControl, | ||
Toolbar, | ||
withNotices, | ||
|
@@ -24,7 +27,6 @@ import { | |
InnerBlocks, | ||
InspectorControls, | ||
MediaPlaceholder, | ||
MediaUpload, | ||
MediaUploadCheck, | ||
PanelColorSettings, | ||
withColors, | ||
|
@@ -42,6 +44,7 @@ import { | |
backgroundImageStyles, | ||
dimRatioToClass, | ||
} from './shared'; | ||
import { speak } from '@wordpress/a11y'; | ||
|
||
/** | ||
* Module Constants | ||
|
@@ -64,14 +67,27 @@ function retrieveFastAverageColor() { | |
} | ||
|
||
class CoverEdit extends Component { | ||
constructor() { | ||
constructor( { attributes } ) { | ||
super( ...arguments ); | ||
this.state = { | ||
isDark: false, | ||
isEditing: ! attributes.url, | ||
}; | ||
this.imageRef = createRef(); | ||
this.videoRef = createRef(); | ||
this.changeIsDarkIfRequired = this.changeIsDarkIfRequired.bind( this ); | ||
this.toggleIsEditing = this.toggleIsEditing.bind( this ); | ||
} | ||
|
||
toggleIsEditing() { | ||
this.setState( { | ||
isEditing: ! this.state.isEditing, | ||
} ); | ||
if ( this.state.isEditing ) { | ||
speak( __( 'You are now viewing the image in the image block.' ) ); | ||
} else { | ||
speak( __( 'You are now editing the image in the image block.' ) ); | ||
} | ||
} | ||
|
||
componentDidMount() { | ||
|
@@ -83,11 +99,11 @@ class CoverEdit extends Component { | |
} | ||
|
||
render() { | ||
const { isEditing } = this.state; | ||
const { | ||
attributes, | ||
setAttributes, | ||
className, | ||
noticeOperations, | ||
noticeUI, | ||
overlayColor, | ||
setOverlayColor, | ||
|
@@ -97,9 +113,23 @@ class CoverEdit extends Component { | |
dimRatio, | ||
focalPoint, | ||
hasParallax, | ||
id, | ||
url, | ||
id, | ||
} = attributes; | ||
|
||
const onSelectUrl = ( newURL ) => { | ||
if ( newURL !== url ) { | ||
this.props.setAttributes( { | ||
url: newURL, | ||
id: undefined, | ||
} ); | ||
} | ||
|
||
this.setState( { | ||
isEditing: false, | ||
} ); | ||
}; | ||
|
||
const onSelectMedia = ( media ) => { | ||
if ( ! media || ! media.url ) { | ||
setAttributes( { url: undefined, id: undefined } ); | ||
|
@@ -134,6 +164,10 @@ class CoverEdit extends Component { | |
{} | ||
), | ||
} ); | ||
|
||
this.setState( { | ||
isEditing: false, | ||
} ); | ||
}; | ||
|
||
const toggleParallax = () => { | ||
|
@@ -157,25 +191,20 @@ class CoverEdit extends Component { | |
style.backgroundPosition = `${ focalPoint.x * 100 }% ${ focalPoint.y * 100 }%`; | ||
} | ||
|
||
const editImageIcon = ( <SVG width={ 20 } height={ 20 } viewBox="0 0 20 20"><Rect x={ 11 } y={ 3 } width={ 7 } height={ 5 } rx={ 1 } /><Rect x={ 2 } y={ 12 } width={ 7 } height={ 5 } rx={ 1 } /><Path d="M13,12h1a3,3,0,0,1-3,3v2a5,5,0,0,0,5-5h1L15,9Z" /><Path d="M4,8H3l2,3L7,8H6A3,3,0,0,1,9,5V3A5,5,0,0,0,4,8Z" /></SVG> ); | ||
const controls = ( | ||
<Fragment> | ||
<BlockControls> | ||
{ !! url && ( | ||
<Fragment> | ||
<MediaUploadCheck> | ||
<Toolbar> | ||
<MediaUpload | ||
onSelect={ onSelectMedia } | ||
allowedTypes={ ALLOWED_MEDIA_TYPES } | ||
value={ id } | ||
render={ ( { open } ) => ( | ||
<IconButton | ||
className="components-toolbar__control" | ||
label={ __( 'Edit media' ) } | ||
icon="edit" | ||
onClick={ open } | ||
/> | ||
) } | ||
<IconButton | ||
className={ classnames( 'components-icon-button components-toolbar__control', { 'is-active': this.state.isEditing } ) } | ||
aria-pressed={ this.state.isEditing } | ||
label={ __( 'Edit media' ) } | ||
icon={ editImageIcon } | ||
onClick={ this.toggleIsEditing } | ||
/> | ||
</Toolbar> | ||
</MediaUploadCheck> | ||
|
@@ -225,25 +254,36 @@ class CoverEdit extends Component { | |
</Fragment> | ||
); | ||
|
||
if ( ! url ) { | ||
const placeholderIcon = <BlockIcon icon={ icon } />; | ||
const label = __( 'Cover' ); | ||
if ( isEditing || ! url ) { | ||
const labels = { | ||
title: __( 'Cover' ), | ||
instructions: __( 'Drag an image or a video, upload a new one or select a file from your library.' ), | ||
}; | ||
|
||
const mediaPreview = ( !! url && <img | ||
alt={ __( 'Edit image' ) } | ||
title={ __( 'Edit image' ) } | ||
className={ 'edit-image-preview' } | ||
src={ url } | ||
/> ); | ||
|
||
return ( | ||
<Fragment> | ||
{ controls } | ||
<MediaPlaceholder | ||
icon={ placeholderIcon } | ||
icon={ <BlockIcon icon={ icon } /> } | ||
className={ className } | ||
labels={ { | ||
title: label, | ||
instructions: __( 'Drag an image or a video, upload a new one or select a file from your library.' ), | ||
} } | ||
labels={ labels } | ||
onSelect={ onSelectMedia } | ||
onSelectURL={ onSelectUrl } | ||
onDoubleClick={ this.toggleIsEditing } | ||
onCancel={ !! url && this.toggleIsEditing } | ||
notices={ noticeUI } | ||
onError={ this.onUploadError } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this right? I couldn't see an onUploadError method. |
||
accept="image/*,video/*" | ||
allowedTypes={ ALLOWED_MEDIA_TYPES } | ||
notices={ noticeUI } | ||
onError={ noticeOperations.createErrorNotice } | ||
value={ { id, url } } | ||
mediaPreview={ backgroundType === IMAGE_BACKGROUND_TYPE && mediaPreview } | ||
/> | ||
</Fragment> | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function should probably be renamed to something like
toggleEditing
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also noticed this function doesn't
speak
when switching while some of the other blocks do.