Add cover image #879

georgeh · 2017-05-23T18:56:53Z

Addresses #487

TODO

Remove text formatting options
Add full-height/parallax options
~~Pre-select image in image chooser when replacing the image~~

mtias · 2017-05-23T19:31:10Z

post-content.js

+			'<!-- /wp:core/cover-image -->',
+
+			'<!-- wp:core/heading -->',
+			'<h1>Welcome to the Gutenberg Editor</h1>',


This is no longer part of the demo content.

Thanks, must have been a merge conflict.

youknowriad

Thanks for your help on this block. I left some small notes

youknowriad · 2017-05-24T11:15:36Z

blocks/library/cover-image/index.js

+					</div>
+					<Button isLarge>
+						{ wp.i18n.__( 'Insert from Media Library' ) }
+					</Button>


The media library integration is already available through the MediaUploadButton. see https://github.com/WordPress/gutenberg/blob/master/blocks/library/image/index.js#L89

We need to start thinking about documenting these (editable, media button, the iframe thing).

youknowriad · 2017-05-24T11:16:58Z

blocks/library/cover-image/index.js

+			title: wp.i18n.__( 'Change Image' ),
+			isActive: ( { align } ) => 'left' === align,
+			onClick: toggleAlignment( 'left' ),
+		},


What is this control for? should we drop it?

youknowriad · 2017-05-24T11:20:52Z

blocks/library/cover-image/index.js

+	category: 'common',
+
+	attributes: {
+		url: attr( 'section', 'data-url' ),


FWIW, it's possible to drop the url attribute here and the url will be saved in the block opening comment.

youknowriad · 2017-05-24T11:22:10Z

blocks/library/cover-image/index.js

+import { registerBlock, query } from '../../api';
+import Editable from '../../editable';
+// TODO: Revisit when we have a common components solution
+import Dashicon from '../../../components/dashicon';


Should we move this to the WordPress dependencies import Dashicon from 'components/dashicon';?

georgeh · 2017-05-24T13:28:08Z

@mtias and @youknowriad thank you for the 👀 , the feedback is really helpful for me (as someone less familiar with the project). I'm still getting the block working, and after that I'll work on cleaning everything up.

georgeh · 2017-06-09T15:02:15Z

@jasmussen would you kindly try this block out and let me know how it feels? I'm still working on a parallax setting but the core functionality should be there

jasmussen · 2017-06-09T16:12:52Z

I like it! I have a bunch of small things, including a different UI for adding/changing the image, but I think we might want to try and keep things minimal for the first merge, then iterate.

There isn't any background-attachment: fixed, though — any reason why? I think that'd be a nice feature to have.

georgeh · 2017-06-12T15:47:32Z

@jasmussen I wanted to make sure I was on the right path before going too far down it. I'm going to play with a few settings for background-attachment: fixed to see what I like

georgeh · 2017-06-19T13:57:07Z

@youknowriad could you take another look at this? I'm ready to merge

mkaz · 2017-06-21T19:42:54Z

Backspace doesn't work in title field
I think I get the Pin for background fixed, but it seems to also align-right, should pin be separate then the alignments - ie. I can pin a full bleed image

georgeh · 2017-06-21T19:56:45Z

@mkaz thanks, I'll check out the backspace.

For alignment vs pin I see the argument for allowing something to be both aligned and pinned. I'll try out a few CSS tweaks to see how they look.

youknowriad · 2017-06-22T10:09:01Z

blocks/library/cover-image/index.js

+/**
+ * WordPress dependencies
+ */
+import Placeholder from 'components/placeholder';


We should use import { Placeholder } from 'components'; to avoid duplication in the built files. (We should probably make this clearer in our dev docs)

youknowriad · 2017-06-22T10:10:53Z

blocks/library/cover-image/index.js

+		title: text( 'h2' ),
+	},
+
+	controls: [


If you rebase master, you'll notice this property doesn't work anymore and I suggest using <BlockAlignmentToolbar> for alignment controls. See #1019 for examples.

youknowriad · 2017-06-22T10:12:06Z

blocks/library/cover-image/index.js

+};
+
+registerBlockType( 'core/cover-image', {
+	title: wp.i18n.__( 'Cover Image' ),


We're trying to avoid using the wp global variable as much as we can in favor of import { __ } from 'i18n. See #1332 for context on this.

youknowriad · 2017-06-22T10:13:26Z

blocks/library/cover-image/index.js

+
+	editFrame.on( 'insert', updateFn );
+	editFrame.open( 'cover-image' );
+};


This is the second block allowing to edit a media. I wonder if should try to factorize this in MediaUploadButton or similar. This could be done in a separate PR.

I'm going to remove this for now but I think it may make sense as a follow-up as it's own InspectorControl.

youknowriad · 2017-06-22T10:16:59Z

blocks/library/cover-image/index.js

+						buttonProps={ uploadButtonProps }
+						onSelect={ setMediaUrl }
+						type="image"
+						auto-open


auto-open has been rename autoOpen

mtias · 2017-06-22T15:27:07Z

@georgeh we should try to use git rebase instead of merges from master.

mtias · 2017-06-22T15:55:30Z

blocks/block-alignment-toolbar/index.js

@@ -25,6 +25,10 @@ const BLOCK_ALIGNMENTS_CONTROLS = {
 		icon: 'align-full-width',
 		title: __( 'Full width' ),
 	},
+	fixed: {


I think this should be a toggle in the inspector, not a shared alignment.

I'm going to make a CheckboxControl to use in the inspector

Should probably just use the existing toggle component for this (see the drop cap setting for text). cc @jasmussen

Looking at the code for TextboxControl it looks like I can pass type="checkbox" - how do you feel about me renaming that to InputControl?

Yes if we were to make a toggle for this it should just be the existing switch:

But I think it's okay to actually remove this advanced property for now to just get the PR merged, if that's simpler.

Alternately, I feel pretty strongly that the fixed background should be applied by default. And it should not be a data-align value. It should be something you can toggle in addition to the image alignment.

Yes, it's not an alignment, just an extra attribute.

I gave George the go-ahead to merge this without this fixed position toggle (which we just removed). I think it's good to merge this block, and we can add the toggle separately if we desire it.

@mkaz

Thanks to @mkaz for the pairing session to get it done

Now using the new `<BlockControls>` to manage the image position. Also removed the ability to change the picture pending moving it into the inspector Also addressed changes in the PR to remove `wp` global and fix autoOpen the MediaUploadButton

- Make background attachment fixed by default. Let's either remove the option to pin it entirely, or move it to the inspector and make it something that's on by default. - Added gray scrim, removed text shadow, reduced font size - Removed alignments because these are now inherited Future improvements we should also consider: add the full-wide button that images have now, and make it so the dark scrim is only present when there's text.

jasmussen · 2017-06-22T18:11:19Z

Okay I think this is good to go! 👍 👍

StaggerLeee · 2017-08-03T13:20:31Z

May I suggest you make Caption field (Cover Image Title) a bit different than rest of the image ?
To make it clear it is a editable element inside image. Right now only visual help is a mouse cursor.

mtias reviewed May 23, 2017

View reviewed changes

youknowriad reviewed May 24, 2017

View reviewed changes

georgeh force-pushed the add/487-cover-image branch from b174020 to ed8fb8e Compare June 6, 2017 17:58

georgeh changed the title ~~WIP Add cover image~~ Add cover image Jun 19, 2017

georgeh added the [Feature] Blocks Overall functionality of blocks label Jun 20, 2017

georgeh added this to the Beta milestone Jun 20, 2017

jasmussen modified the milestones: Beta, Beta 2 Jun 21, 2017

youknowriad reviewed Jun 22, 2017

View reviewed changes

mtias reviewed Jun 22, 2017

View reviewed changes

George Hotelling added 10 commits June 22, 2017 13:27

Cover Image is showing up in the editor… kinda…

6e045c7

background image not showing up

e54c0e7

Fixed background image style

c667218

Clean up and remove outdated code

14c006e

Use MediaUploadButton

76d80ce

Cover images working in the editor

a0ff9d7

Thanks to @mkaz for the pairing session to get it done

registerBlock ➡️ registerBlockType

75964d8

Fix tests

e3c899b

Remove formatting options for title

36a9f5e

Add background-attachment: fixed view

dd241d9

George Hotelling and others added 3 commits June 22, 2017 13:27

Pre-selecting image by URL doesn't make sense

41f9966

Control updates

bf18706

Now using the new `<BlockControls>` to manage the image position. Also removed the ability to change the picture pending moving it into the inspector Also addressed changes in the PR to remove `wp` global and fix autoOpen the MediaUploadButton

georgeh force-pushed the add/487-cover-image branch from cb1f6ac to 6985c7b Compare June 22, 2017 17:28

George Hotelling added 2 commits June 22, 2017 13:29

Re-introduce post-content data

336b453

Add cover-image back into list

99fb455

mtias removed this from the Beta 2 milestone Jun 22, 2017

George Hotelling and others added 2 commits June 22, 2017 14:02

Removing "fixed" alignment

bfcbe7d

Fix full bleed issues.

59f89b1

georgeh merged commit b995440 into master Jun 22, 2017

ellatrix deleted the add/487-cover-image branch July 2, 2017 08:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cover image #879

Add cover image #879

georgeh commented May 23, 2017 •

edited

Loading

mtias May 23, 2017

georgeh May 24, 2017

youknowriad left a comment

youknowriad May 24, 2017

mtias May 29, 2017

youknowriad May 24, 2017

youknowriad May 24, 2017

youknowriad May 24, 2017

georgeh commented May 24, 2017

georgeh commented Jun 9, 2017

jasmussen commented Jun 9, 2017

georgeh commented Jun 12, 2017

georgeh commented Jun 19, 2017

mkaz commented Jun 21, 2017

georgeh commented Jun 21, 2017

youknowriad Jun 22, 2017

youknowriad Jun 22, 2017

youknowriad Jun 22, 2017

youknowriad Jun 22, 2017

georgeh Jun 22, 2017

youknowriad Jun 22, 2017

mtias commented Jun 22, 2017

mtias Jun 22, 2017 •

edited

Loading

georgeh Jun 22, 2017

mtias Jun 22, 2017 •

edited

Loading

georgeh Jun 22, 2017

jasmussen Jun 22, 2017

mtias Jun 22, 2017

jasmussen Jun 22, 2017

jasmussen commented Jun 22, 2017

StaggerLeee commented Aug 3, 2017

Add cover image #879

Add cover image #879

Conversation

georgeh commented May 23, 2017 • edited Loading

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

georgeh commented May 24, 2017

georgeh commented Jun 9, 2017

jasmussen commented Jun 9, 2017

georgeh commented Jun 12, 2017

georgeh commented Jun 19, 2017

mkaz commented Jun 21, 2017

georgeh commented Jun 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtias commented Jun 22, 2017

mtias Jun 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtias Jun 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasmussen commented Jun 22, 2017

StaggerLeee commented Aug 3, 2017

georgeh commented May 23, 2017 •

edited

Loading

mtias Jun 22, 2017 •

edited

Loading

mtias Jun 22, 2017 •

edited

Loading