-
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
Reusable Blocks: Support importing and exporting reusable blocks #9788
Changes from 5 commits
e1d5ab4
3ad4d09
41d7454
41a2ac5
065a008
0b58955
34d4b4d
89ef949
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 |
---|---|---|
@@ -0,0 +1 @@ | ||
package-lock=false |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# Reusable blocks listing page | ||
|
||
Package used to add import/export links to the listing page of the reusable blocks. | ||
|
||
<br/><br/><p align="center"><img src="https://s.w.org/style/images/codeispoetry.png?1" alt="Code is Poetry." /></p> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
{ | ||
"name": "@wordpress/list-reusable-blocks", | ||
"version": "1.0.0", | ||
"description": "Adding Export/Import support to the reusable blocks listing.", | ||
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. Really stress-testing our policy of publishing all the things as modules, huh 😅 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. I made it private :) 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. Now that I think about it. Making it private means it can't be included in Core as an external package and begs the question of whether it should be moved to Core or kept in the repo post-merge. 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. I mean, I never said I was opposed to it being published to npm 😉 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. yes, I know it was private even before the comment :) |
||
"author": "The WordPress Contributors", | ||
"license": "GPL-2.0-or-later", | ||
"keywords": [ | ||
"templates", | ||
"reusable blocks" | ||
], | ||
"private": true, | ||
"homepage": "https://github.com/WordPress/gutenberg/tree/master/packages/list-reusable-blocks/README.md", | ||
"repository": { | ||
"type": "git", | ||
"url": "https://github.com/WordPress/gutenberg.git" | ||
}, | ||
"bugs": { | ||
"url": "https://github.com/WordPress/gutenberg/issues" | ||
}, | ||
"main": "build/index.js", | ||
"module": "build-module/index.js", | ||
"dependencies": { | ||
"@babel/runtime": "^7.0.0", | ||
"@wordpress/api-fetch": "file:../api-fetch", | ||
"@wordpress/components": "file:../components", | ||
"@wordpress/compose": "file:../compose", | ||
"@wordpress/element": "file:../element", | ||
"@wordpress/i18n": "file:../i18n", | ||
"lodash": "^4.17.10" | ||
}, | ||
"publishConfig": { | ||
"access": "public" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { flow } from 'lodash'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { Dropdown, Button } from '@wordpress/components'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import ImportForm from '../import-form'; | ||
|
||
function ImportDropdown( { onUpload } ) { | ||
return ( | ||
<Dropdown | ||
position="bottom right" | ||
contentClassName="list-reusable-blocks-import-dropdown__content" | ||
renderToggle={ ( { isOpen, onToggle } ) => ( | ||
<Button | ||
type="button" | ||
aria-expanded={ isOpen } | ||
onClick={ onToggle } | ||
isPrimary | ||
> | ||
{ __( 'Import from JSON' ) } | ||
</Button> | ||
) } | ||
renderContent={ ( { onClose } ) => ( | ||
<ImportForm onUpload={ flow( onClose, onUpload ) } /> | ||
) } | ||
/> | ||
); | ||
} | ||
|
||
export default ImportDropdown; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
.list-reusable-blocks-import-dropdown__content .components-popover__content { | ||
padding: 10px; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { Component } from '@wordpress/element'; | ||
import { withInstanceId } from '@wordpress/compose'; | ||
import { __ } from '@wordpress/i18n'; | ||
import { Button, Notice } from '@wordpress/components'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import importReusableBlock from '../../utils/import'; | ||
|
||
class ImportForm extends Component { | ||
constructor() { | ||
super( ...arguments ); | ||
this.state = { | ||
isLoading: false, | ||
error: null, | ||
file: null, | ||
}; | ||
|
||
this.isMounted = true; | ||
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. Re: Previous comment, this is actually a part of the React component API, though discouraged, which may explain errors around attempts to set it. https://reactjs.org/blog/2015/12/16/ismounted-antipattern.html 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. I know the antipattern but sometimes I'm pragmatic and don't want to introduce more complex solutions like a cancelable promise or something like that. Especially since it's a small JS file for a separate page here. 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, noting that I wasn't able to reproduce any of the errors you had, so I'd appreciate another check 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. The point was less about it being discouraged / deprecated, and more to the point that this is already a property defined on a default React component, and we're overriding it. |
||
this.onChangeFile = this.onChangeFile.bind( this ); | ||
this.onSubmit = this.onSubmit.bind( this ); | ||
} | ||
|
||
componentWillUnmount() { | ||
this.isMounted = false; | ||
} | ||
|
||
onChangeFile( event ) { | ||
this.setState( { file: event.target.files[ 0 ] } ); | ||
} | ||
|
||
onSubmit( event ) { | ||
event.preventDefault(); | ||
const { file } = this.state; | ||
const { onUpload } = this.props; | ||
if ( ! file ) { | ||
return; | ||
} | ||
this.setState( { isLoading: true } ); | ||
importReusableBlock( file ) | ||
.then( ( reusableBlock ) => { | ||
if ( ! this.isMounted ) { | ||
return; | ||
} | ||
|
||
this.setState( { isLoading: false } ); | ||
onUpload( reusableBlock ); | ||
} ) | ||
.catch( ( error ) => { | ||
if ( ! this.isMounted ) { | ||
return; | ||
} | ||
|
||
this.setState( { isLoading: false, error: error.message } ); | ||
} ); | ||
} | ||
|
||
render() { | ||
const { instanceId } = this.props; | ||
const { file, isLoading, error } = this.state; | ||
const inputId = 'list-reusable-blocks-import-form-' + instanceId; | ||
return ( | ||
<form | ||
className="list-reusable-blocks-import-form" | ||
onSubmit={ this.onSubmit } | ||
> | ||
{ error && ( | ||
<Notice status="error"> | ||
{ error } | ||
</Notice> | ||
) } | ||
<label | ||
htmlFor={ inputId } | ||
className="list-reusable-blocks-import-form__label" | ||
> | ||
{ __( 'File' ) } | ||
</label> | ||
<input | ||
id={ inputId } | ||
type="file" | ||
onChange={ this.onChangeFile } | ||
/> | ||
<Button | ||
type="submit" | ||
isBusy={ isLoading } | ||
disabled={ ! file || isLoading } | ||
isDefault | ||
className="list-reusable-blocks-import-form__button" | ||
> | ||
{ __( 'Import' ) } | ||
</Button> | ||
</form> | ||
); | ||
} | ||
} | ||
|
||
export default withInstanceId( ImportForm ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
.list-reusable-blocks-import-form__label { | ||
display: block; | ||
margin-bottom: 10px; | ||
} | ||
|
||
.list-reusable-blocks-import-form__button { | ||
margin-top: 20px; | ||
float: right; | ||
} | ||
|
||
.list-reusable-blocks-import-form .components-notice__content { | ||
margin: 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { render } from '@wordpress/element'; | ||
import { __ } from '@wordpress/i18n'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import exportReusableBlock from './utils/export'; | ||
import ImportDropdown from './components/import-dropdown'; | ||
|
||
// Setup Export Links | ||
document.body.addEventListener( 'click', ( event ) => { | ||
if ( ! event.target.classList.contains( 'wp-list-reusable-blocks__export' ) ) { | ||
return; | ||
} | ||
event.preventDefault(); | ||
exportReusableBlock( event.target.dataset.id ); | ||
} ); | ||
|
||
// Setup Import Form | ||
document.addEventListener( 'DOMContentLoaded', function() { | ||
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. Inconsistency: |
||
const button = document.querySelector( '.page-title-action' ); | ||
if ( ! button ) { | ||
return; | ||
} | ||
|
||
const showNotice = () => { | ||
const notice = document.createElement( 'div' ); | ||
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. Could we render a 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. I did try it, the issue is that it's styling is very different from the regular notices in this kind of pages. We should reconsider if we rewrite the whole page in JS |
||
notice.className = 'notice notice-success is-dismissible'; | ||
notice.innerHTML = `<p>${ __( 'Reusable block imported with success!' ) }</p>`; | ||
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. Grammar: "successfully" might work better than "with success" here. |
||
|
||
const headerEnd = document.querySelector( '.wp-header-end' ); | ||
if ( ! headerEnd ) { | ||
return; | ||
} | ||
headerEnd.parentNode.insertBefore( notice, headerEnd ); | ||
}; | ||
|
||
const container = document.createElement( 'div' ); | ||
container.className = 'list-reusable-blocks__container'; | ||
button.parentNode.insertBefore( container, button ); | ||
render( <ImportDropdown onUpload={ showNotice } />, container ); | ||
} ); |
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.
Is this convention lifted from somewhere? Doesn't seem obvious why we'd need an
aria-label
which has the same text as the element itself.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.
yes, it's inspired by the way we add the "Classic editor" link in the post list actions