Add media upload button to Galleries #2294

mkaz · 2017-08-08T20:16:02Z

Abstract the media upload button being used for images to utility function and add the same upload button for Gallery block.

Closes #2131

mkaz · 2017-08-08T20:17:47Z

@youknowriad - I committed a checkpoint for the upload button abstracted out to confirm it looks ok. I figured in the utils section made the most sense. I'm working on the multiple upload now.

mkaz · 2017-08-09T22:32:47Z

[ redacted ]

See below.

mkaz · 2017-08-10T00:22:01Z

I think I might of solved a couple of issues with this one now, previous open questions are no longer relevant, sorry for the pings.

The latest update switches out the edit to use the MediaUploadButton which was updated to support value={ ids } when multiple is set, so edit works without needing all that data. Whew!
@joemcgill there shouldn't be any changes required for core for this.

This PR will also close #1986

The test data needs to be updated which is why the build is failing, I'll update that shortly

youknowriad

Left some notes, but this PR is huge improvement for the gallery block

youknowriad · 2017-08-10T10:12:43Z

utils/fileupload.js

@@ -0,0 +1,83 @@
+
+export function fileUpload( files, setAttributes ) {


Can we add some JSDocs to these two functions?
I wonder if we can test them too, maybe by mocking wp.api.models, I understand it's not straightforward tough

youknowriad · 2017-08-10T10:14:38Z

blocks/library/gallery/index.js

+									type="image"
+									multiple="true"
+									gallery="true"
+									value={ images.map( ( img ) => img.id ) }


youknowriad · 2017-08-10T10:15:22Z

blocks/library/gallery/index.js

+									onSelect={ onSelectImages }
+									type="image"
+									multiple="true"
+									gallery="true"


Can't we avoid the ="true" for these two props: multiple and gallery, seems like boolean props to me?

youknowriad · 2017-08-10T10:15:44Z

blocks/library/gallery/index.js

+						className="wp-block-image__upload-button"
+						onChange={ uploadFromFiles }
+						accept="image/*"
+						multiple="true"


same here multiple should be enough

youknowriad · 2017-08-10T10:16:35Z

blocks/test/fixtures/core__gallery.html

@@ -1,4 +1,4 @@
-<!-- wp:core/gallery {"images":[{"sizes":{"full":{"url":"https://cldup.com/uuUqE_dXzy.jpg","height":1080,"width":810,"orientation":"portrait"}},"mime":"image/jpeg","type":"image","subtype":"jpeg","id":1,"url":"https://cldup.com/uuUqE_dXzy.jpg","alt":"title"},{"sizes":{"full":{"url":"http://google.com/hi.png","height":1080,"width":1440,"orientation":"landscape"}},"mime":"image/jpeg","type":"image","subtype":"jpeg","id":2,"url":"http://google.com/hi.png","alt":"title"}]} -->
+<!-- wp:core/gallery -->


Awesome 👍

youknowriad · 2017-08-10T10:18:40Z

utils/fileupload.js

+			] } );
+		} );
+	} );
+}


Could we rename these functions and the inner variables mediaUpload and media instead of fileUpload and img? I'm assuming these are generic functions to upload any media.

I refactored so image and gallery block use the same function, renamed variables and added some comments so hopefully is a bit cleaner

…mes and comments

codecov · 2017-08-11T14:12:47Z

Codecov Report

Merging #2294 into master will increase coverage by 0.08%.
The diff coverage is 25.8%.

@@            Coverage Diff             @@
##           master    #2294      +/-   ##
==========================================
+ Coverage   25.74%   25.82%   +0.08%     
==========================================
  Files         155      156       +1     
  Lines        4821     4829       +8     
  Branches      812      815       +3     
==========================================
+ Hits         1241     1247       +6     
  Misses       3024     3024              
- Partials      556      558       +2

Impacted Files	Coverage Δ
blocks/library/image/index.js	`15.38% <0%> (+3.13%)`	⬆️
blocks/library/gallery/gallery-image.js	`50% <100%> (ø)`	⬆️
utils/mediaupload.js	`21.73% <21.73%> (ø)`
blocks/library/gallery/index.js	`33.33% <33.33%> (+8.33%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a86b79d...2d88d13. Read the comment docs.

mkaz self-assigned this Aug 8, 2017

mkaz added the [Status] In Progress Tracking issues with work in progress label Aug 8, 2017

mkaz mentioned this pull request Aug 9, 2017

Allow media upload using drag and drop #1563

Closed

mkaz force-pushed the add/gallery-upload-button branch from 21033eb to 4388396 Compare August 10, 2017 00:02

WordPress deleted a comment from codecov bot Aug 10, 2017

youknowriad reviewed Aug 10, 2017

View reviewed changes

mkaz force-pushed the add/gallery-upload-button branch from 9c1902a to 0e2c54b Compare August 10, 2017 15:36

mkaz removed the [Status] In Progress Tracking issues with work in progress label Aug 10, 2017

mtias added [Feature] Blocks Overall functionality of blocks [Feature] Media Anything that impacts the experience of managing media labels Aug 11, 2017

mkaz added 17 commits August 11, 2017 06:46

Add initial abstraction for File Upload

13b6b35

Use abstracted fileUpload function

cb9acac

Remove console debug

726fe5c

Add image upload button for gallery

35f8a61

Checkpoint: files uploading, but not editable in modal

b075215

Add link and id to img object

6dbe9d0

Add id to gallery image

732c4b7

Switch media modal to use MediaUploadButton on edit

cbaa1ef

Value for MediaUploadButton needs to be array of ids

7629485

Fix core gallery test fixtures with simplified data

3ac5392

Add core gallery test for columns parameter

6a600a2

Lint fix trailing comma

7b14eca

Can just use parameter, dont need ="true"

0d6a4dc

Rename fileupload.js to mediaupload.js

d335497

Refactor so image and gallery use same function, clean up variable na…

d050713

…mes and comments

Remove tracking include, rebase error

937026d

Add initial test for mediaupload.js

2d88d13

mkaz force-pushed the add/gallery-upload-button branch from 95daed0 to 2d88d13 Compare August 11, 2017 13:47

mkaz merged commit 6d3637f into master Aug 11, 2017

mkaz deleted the add/gallery-upload-button branch August 11, 2017 14:12

mkaz mentioned this pull request Aug 11, 2017

Gallery: Reduce attribute storage requirements #1986

Closed

youknowriad mentioned this pull request Aug 14, 2017

Image Block: Fix dropping images on the image block #2399

Merged

joemcgill mentioned this pull request Aug 21, 2017

Gallery: Update media frame state management #2488

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add media upload button to Galleries #2294

Add media upload button to Galleries #2294

mkaz commented Aug 8, 2017

mkaz commented Aug 8, 2017

mkaz commented Aug 9, 2017 •

edited

Loading

mkaz commented Aug 10, 2017 •

edited

Loading

youknowriad left a comment

youknowriad Aug 10, 2017

youknowriad Aug 10, 2017

youknowriad Aug 10, 2017

mkaz Aug 10, 2017

youknowriad Aug 10, 2017

mkaz Aug 10, 2017

youknowriad Aug 10, 2017

youknowriad Aug 10, 2017

mkaz Aug 10, 2017

codecov bot commented Aug 11, 2017 •

edited

Loading

		@@ -0,0 +1,83 @@

		export function fileUpload( files, setAttributes ) {

		@@ -1,4 +1,4 @@
		<!-- wp:core/gallery {"images":[{"sizes":{"full":{"url":"https://cldup.com/uuUqE_dXzy.jpg","height":1080,"width":810,"orientation":"portrait"}},"mime":"image/jpeg","type":"image","subtype":"jpeg","id":1,"url":"https://cldup.com/uuUqE_dXzy.jpg","alt":"title"},{"sizes":{"full":{"url":"http://google.com/hi.png","height":1080,"width":1440,"orientation":"landscape"}},"mime":"image/jpeg","type":"image","subtype":"jpeg","id":2,"url":"http://google.com/hi.png","alt":"title"}]} -->
		<!-- wp:core/gallery -->

Add media upload button to Galleries #2294

Add media upload button to Galleries #2294

Conversation

mkaz commented Aug 8, 2017

mkaz commented Aug 8, 2017

mkaz commented Aug 9, 2017 • edited Loading

mkaz commented Aug 10, 2017 • edited Loading

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

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

codecov bot commented Aug 11, 2017 • edited Loading

Codecov Report

mkaz commented Aug 9, 2017 •

edited

Loading

mkaz commented Aug 10, 2017 •

edited

Loading

codecov bot commented Aug 11, 2017 •

edited

Loading