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

Implement image block placeholder #566

Merged
merged 1 commit into from
May 2, 2017
Merged

Implement image block placeholder #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 20 additions & 0 deletions blocks/library/image/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
import './style.scss';
import { registerBlock, query } from 'api';
import Editable from 'components/editable';
// TODO: Revisit when we have a common components solution
import Dashicon from '../../../editor/components/dashicon';
import Button from '../../../editor/components/button';

const { attr, children } = query;

Expand Down Expand Up @@ -78,6 +81,23 @@ registerBlock( 'core/image', {
edit( { attributes, setAttributes, focus, setFocus } ) {
const { url, alt, caption } = attributes;

if ( ! url ) {
return (
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to move this into an empty function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Would it make sense to move this into an empty function?

What does "empty" mean?

Copy link
Member

Choose a reason for hiding this comment

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

It means when it acts as a placeholder, when no content is yet set. This is going to be a prevalent case for templates down the road.

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 means when it acts as a placeholder, when no content is yet set. This is going to be a prevalent case for templates down the road.

Right, but in the sense of treating blocks as rendering always from its state, an imperative "renderNew" doesn't make sense. A block could implement conditions under which it could be considered empty (here, ! url), but it's unclear how we'd want to generalize this. Maybe related to #609, an attribute can be flagged as required and, when missing, use a separate render function like your empty example.

I don't know that introducing this break from edit / save duality really provide a whole lot of value though for what could be equally represented by:

edit: ( { attributes } ) =>
	attributes.url
		? <ImageEdit />
		: <EmptyImageEdit />

Copy link
Member

@mtias mtias May 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

That last example looks alright, but I'm mostly concerned with people doing all sort of different things for something that should have a consistent UX. Empty could be just the comment (leaving blocks to implement more fine grained UI handling when specific attributes are missing), which could be a nice way to define a template:

<!-- wp:core/text --><!-- /wp:core/text -->
<!-- wp:core/image --><!-- /wp:core/image -->
<!-- wp:core/button --><!-- /wp:core/button -->

In any case I think forcing block authors to have a separate function for returning the empty render, regardless of whether it is called from edit manually, could be helpful.

<div className="blocks-image is-placeholder">
<div className="blocks-image__placeholder-label">
<Dashicon icon="format-image" />
{ wp.i18n.__( 'Image' ) }
</div>
<div className="blocks-image__placeholder-instructions">
{ wp.i18n.__( 'Drag image here or insert from media library' ) }
</div>
<Button isLarge>
{ wp.i18n.__( 'Insert from Media Library' ) }
</Button>
</div>
);
}

return (
<figure className="blocks-image">
<img src={ url } alt={ alt } />
Expand Down
22 changes: 22 additions & 0 deletions blocks/library/image/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,25 @@
width: 100%;
}
}

.blocks-image.is-placeholder {
margin: -15px;
padding: 6em 0;
border: 2px solid $light-gray-500;
text-align: center;
}

.blocks-image__placeholder-label {
display: flex;
align-items: center;
justify-content: center;
font-weight: bold;

.dashicon {
margin-right: 1ch;
}
}

.blocks-image__placeholder-instructions {
margin: 1.8em 0;
}
2 changes: 1 addition & 1 deletion editor/modes/visual-editor/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
position: absolute;
top: 0;
bottom: 0;
left: 33px;
left: 35px;
right: 0;
border: 2px solid transparent;
transition: 0.2s border-color;
Expand Down
26 changes: 17 additions & 9 deletions languages/gutenberg.pot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,19 @@ msgid "Strikethrough"
msgstr ""

#: blocks/components/editable/index.js:29
#: blocks/library/image/index.js:41
#: blocks/library/image/index.js:44
#: blocks/library/list/index.js:25
msgid "Align left"
msgstr ""

#: blocks/components/editable/index.js:34
#: blocks/library/image/index.js:47
#: blocks/library/image/index.js:50
#: blocks/library/list/index.js:33
msgid "Align center"
msgstr ""

#: blocks/components/editable/index.js:39
#: blocks/library/image/index.js:53
#: blocks/library/image/index.js:56
#: blocks/library/list/index.js:41
msgid "Align right"
msgstr ""
Expand All @@ -46,7 +46,7 @@ msgid "Embed"
msgstr ""

#: blocks/library/embed/index.js:33
#: blocks/library/image/index.js:87
#: blocks/library/image/index.js:107
msgid "Write caption…"
msgstr ""

Expand All @@ -62,16 +62,24 @@ msgstr ""
msgid "Heading %s"
msgstr ""

#: blocks/library/image/index.js:25
#: blocks/library/image/index.js:62
msgid "No alignment"
msgstr ""

#: blocks/library/image/index.js:68
msgid "Wide width"
msgstr ""

#: blocks/library/image/index.js:89
msgid "Image"
msgstr ""

#: blocks/library/image/index.js:59
msgid "No alignment"
#: blocks/library/image/index.js:92
msgid "Drag image here or insert from media library"
msgstr ""

#: blocks/library/image/index.js:65
msgid "Wide width"
#: blocks/library/image/index.js:95
msgid "Insert from Media Library"
msgstr ""

#: blocks/library/list/index.js:11
Expand Down