Add Gallery Block #740

mkaz · 2017-05-09T18:30:29Z

Initial PR for adding the gallery block - the initial code just stubs out the basics. Will need to add in multiple style galleries, drag-drop reordering, image upload, and any additional styles, will break these into additional smaller issues so it's not all done under one large PR.

See Issue #317

jasmussen · 2017-05-10T07:49:48Z

Thrilled you're working on this! I tried building but it's erroring for me. But I see that it's in progress, let me know if/when you'd like eyes on this.

jasmussen · 2017-05-11T08:03:29Z

Nice first stab!

Can you use the new Placeholder components instead of making the placeholder manually? See also embed.js for an example. I know the design is slightly different.

It would be really good to hook up the media library, I think lots of pieces fall in place then.

🎉

mkaz · 2017-05-22T19:34:47Z

@jasmussen Media Library integrated, check it out. Any suggestions on removing images already added or adding more? We could click the whole block and take back into Media Library?

westonruter · 2017-05-22T19:56:54Z

Cross reference core Gallery widget: xwp/wp-core-media-widgets#120

jasmussen · 2017-05-23T09:36:26Z

Media Library integrated, check it out. Any suggestions on removing images already added or adding more? We could click the whole block and take back into Media Library?

I tweaked/updated some mockups to explore this better.

Neutral:

Selected:

To explain the blueprint — the toolbar that shows up on the block itself is the "Quick Toolbar". The "Edit" button on that toolbar, same as for the gallery block in WordPress currently, opens the media library on the "Edit Gallery" page, that looks like this:

The sidebar on the right, is the "Inspector". The intricate details here are still a little bit in flux, and subject to change, so these are best handled in a separate PR. But essentially these contain more advanced aspects of editing the block, than can fit in the quick toolbar.

klihelp · 2017-05-23T16:25:31Z

looks good:)

mkaz · 2017-05-23T17:41:14Z

I understand for the Quick Toolbar to include the Edit which will go back to Media Library, and the drop down to pick the different styles. However, I'm not sure what the alignment (left, right, center, and stretch) means for a gallery

jasmussen · 2017-05-23T18:49:50Z

However, I'm not sure what the alignment (left, right, center, and stretch) means for a gallery

So keep in mind we're building desktop publishing for the web. And so it makes sense to think of a gallery more as "a blob of images" than a traditional gallery, which is just a kitchen sink full of photos.

For example, imagine a gallery with 3 pictures, set to 1 column in the options, floated left, with text flowing around it on the right.

Or imagine a full-width/stretch'ed gallery set to 3 columns, as a separator between two paragraphs.

The alignments are basically "layout tools", and users can do anything with them they like. Though ideally we'd be able to re-use the alignments from the image block 1:1 without much in the way of changing it.

Some quick and dirty mockups... align none:

Align full width:

nylen · 2017-05-26T01:04:14Z

After #849 which added a bunch of tests for block parsing and serialization, this PR should probably be rebased against master, either because the build will fail upon merge, or because this PR can add some new tests using this structure (details here).

mkaz · 2017-05-30T22:09:30Z

Hi @nylen - I've rebased and updated the block including the new text fixture.
I'm a little stuck trying to get it to generate the .json and .serialized files - could you help by taking a look.

 1) full post content fixture core-gallery:
     TypeError: Cannot read property 'children' of undefined

I'm seeing the above error. Once the tests part get straightened out, the block should be ready to go - so can use a review too :-)

nylen · 2017-05-31T18:33:45Z

@mkaz the issues with the test framework itself are addressed in the latest few commits. However, these tests are pointing to some problems with the block. The markup in the core-gallery.html fixture does not have the format expected by the attributes matchers:

url: attr( 'data-url' ),
alt: attr( 'data-alt' ),

As a result, these attributes are passed through the block as { url: undefined, alt: undefined }, which is converted to {} when serialized to JSON. Here is the resulting JSON fixture file, which should contain details about the images instead:

[
    {
        "uid": "_uid_0",
        "blockType": "core/gallery",
        "attributes": {
            "images": [
                {},
                {}
            ]
        }
    }
]

Previously, any object passing through that part of the test code was assumed to be a React component, which was causing the specific issue you were seeing. I've fixed that.

Separately, I don't think we should duplicate the URL and alt in the data-url and data-alt attributes at all. Why not just grab them straight from the img tag?

mkaz · 2017-05-31T19:01:21Z

Thanks @nylen - I thought I saw somewhere it was preferred to include the attributes in data, I'll switch to get them out of the img tag itself

Creates initial gallery block, with stubbed out functionality. Will create tickets for drag/drop, upload, multiple styles

@georgeh

Instead of reopening the Media Gallery, which the Media Upload Button does, when editing the gallery requires opening the Edit Gallery modal. Also requires setting selection and event which translates backbone data. Props to @georgeh, who I paired with developing See wp.media.editor from wp-includes/js/media-views.js

mtias · 2017-06-07T19:15:32Z

blocks/library/gallery/gallery-image.js

@@ -0,0 +1,10 @@
+
+export default class GalleryImage extends wp.element.Component {


Dos this need to be an element.Component or could it be just a function?

mtias · 2017-06-07T19:17:08Z

blocks/test/fixtures/core-gallery.html

@@ -0,0 +1,10 @@
+<!-- wp:core/gallery -->
+<div class="blocks-gallery">
+	<div class="blocks-gallery-image">


These could be figure instead of div for the markup.

aduth · 2017-06-07T20:19:58Z

blocks/library/gallery/gallery-image.js

+export default class GalleryImage extends wp.element.Component {
+	render() {
+		return (
+			<div key={ this.props.i } className="blocks-gallery-image">


key here isn't doing anything.

This wasn't addressed.

aduth · 2017-06-07T20:22:54Z

blocks/library/gallery/index.js

+					<MediaUploadButton
+						onSelect={ setMediaUrl }
+						type="image"
+						auto-open


Curious why we chose dash-separated here, instead of autoOpen. Camel-case to be the convention with prop naming.

This is how it was specified in MediaUploadButton, I'd prefer to ship and then fix in another PR since it touches a couple blocks at once. Created issue #1083 to handle

aduth · 2017-06-07T20:24:53Z

blocks/library/gallery/index.js

+		} );
+	}
+
+	editFrame.on( 'insert', updateFn );


Do we need to detach these event handlers after the edit frame is closed, to avoid risk of zombie event listeners?

On the frame Unmount the entire frame is destroyed, so the event handlers should be taken care of with that. See https://github.com/WordPress/gutenberg/blob/master/blocks/media-upload-button/index.js#L36

mkaz · 2017-06-07T20:52:06Z

Thanks @mtias updated with your suggestions and switched out to simple function instead of component 👍

mtias · 2017-06-08T22:44:43Z

Thanks! Let's get it in and iterate.

jasmussen · 2017-06-09T14:55:09Z

🎉

StaggerLeee · 2017-08-17T07:22:23Z

Would like to ask for one non-GUI option for Admins, filter. To be able to limit max number of Items per gallery ?
Actually two in one, to be able to limit max number of galleries per Page/Post.

What is your advice, how can we continue to call "filters" now in React framework ?

mkaz · 2017-08-18T19:35:05Z

@StaggerLeee Are you looking for an option to limit a user on how many items they can put into a gallery? Is this necessary?

I think something like that could easily be circumnavigated by inserting two gallery blocks into the same post, or just inserting multiple images.

Can you create a new issue for this discussion?

StaggerLeee · 2017-08-18T22:05:49Z

Maybe in some later stage. Now you all have to much to do.

I meant you moved everything to blocks, all widgets. When they are used on Homepage template could be nice to have some limit option, and still give User power to change things on Homepage.

mkaz self-assigned this May 9, 2017

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

This was referenced May 9, 2017

Gallery Block: reordering images, including support for Drag&Drop #743

Closed

Gallery Block: Add Multiple Styles #744

Closed

mkaz force-pushed the add/317-gallery-block branch from 2039eb9 to 62ef812 Compare May 10, 2017 17:26

mkaz force-pushed the add/317-gallery-block branch from ce749ba to 91fec76 Compare May 11, 2017 18:03

mkaz force-pushed the add/317-gallery-block branch 2 times, most recently from b7e9be4 to 0e25d20 Compare May 22, 2017 19:31

westonruter mentioned this pull request May 22, 2017

Gallery Widget xwp/wp-core-media-widgets#120

Merged

mkaz force-pushed the add/317-gallery-block branch from 0786012 to 2787e14 Compare May 30, 2017 22:06

mkaz added [Feature] Blocks Overall functionality of blocks and removed [Status] In Progress Tracking issues with work in progress labels May 31, 2017

mkaz requested a review from nylen May 31, 2017 19:31

mkaz force-pushed the add/317-gallery-block branch 2 times, most recently from 30e366b to d6d4ec0 Compare June 5, 2017 16:13

mkaz added 3 commits June 5, 2017 09:34

Initial gallery block

bc92ce1

Creates initial gallery block, with stubbed out functionality. Will create tickets for drag/drop, upload, multiple styles

Add gallery to library list

b87177a

Fix paths for imports

7d59acc

mkaz and others added 11 commits June 5, 2017 09:34

Switch out with Placeholder component

a77e250

Integrate Media Library button into Gallery

0303d84

Add controls for alignment, and start to edit opening back media library

5f9b88a

Change CSS to just blocks-gallery to fix image width issue

02cadf4

Fix query/set attributes

9be7823

Initial fixture for testing

da0fe96

Work around receiving objects that are not React components

26f5d77

Add remaining core/gallery test fixtures

dc9412a

Switch attributes to use img attributes and not add data attributes

0aee746

Fix to registerBlockType

c3cbce6

mkaz force-pushed the add/317-gallery-block branch from d6d4ec0 to c3cbce6 Compare June 5, 2017 16:50

mtias reviewed Jun 7, 2017

View reviewed changes

aduth reviewed Jun 7, 2017

View reviewed changes

mkaz added 3 commits June 7, 2017 13:49

Update test to use <figure> instead of <div> for images

469fc55

Switch attributes to use figure for querying

263c401

Switch to simple function, use <figure> instead of <div>

a7108ba

Merge branch 'master' into add/317-gallery-block

f3b4386

mkaz mentioned this pull request Jun 8, 2017

Rename auto-open prop to autoOpen in MediaUploadButton #1083

Closed

mkaz merged commit 749c69f into master Jun 9, 2017

mkaz deleted the add/317-gallery-block branch June 9, 2017 14:52

westonruter mentioned this pull request Jun 11, 2017

Add Block: Gallery #317

Closed

yaseenaljamal mentioned this pull request Sep 13, 2023

[Snyk] Upgrade valtio from 1.7.0 to 1.11.2 yaseenaljamal/gutenberg#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Gallery Block #740

Add Gallery Block #740

mkaz commented May 9, 2017

jasmussen commented May 10, 2017

jasmussen commented May 11, 2017

mkaz commented May 22, 2017

westonruter commented May 22, 2017

jasmussen commented May 23, 2017

klihelp commented May 23, 2017

mkaz commented May 23, 2017

jasmussen commented May 23, 2017

nylen commented May 26, 2017

mkaz commented May 30, 2017

nylen commented May 31, 2017

mkaz commented May 31, 2017

mtias Jun 7, 2017

mtias Jun 7, 2017

aduth Jun 7, 2017

aduth Jun 9, 2017

aduth Jun 7, 2017

mkaz Jun 8, 2017

aduth Jun 7, 2017

mkaz Jun 8, 2017

mkaz commented Jun 7, 2017

mtias commented Jun 8, 2017

jasmussen commented Jun 9, 2017

StaggerLeee commented Aug 17, 2017

mkaz commented Aug 18, 2017

StaggerLeee commented Aug 18, 2017

		@@ -0,0 +1,10 @@

		export default class GalleryImage extends wp.element.Component {

Add Gallery Block #740

Add Gallery Block #740

Conversation

mkaz commented May 9, 2017

jasmussen commented May 10, 2017

jasmussen commented May 11, 2017

mkaz commented May 22, 2017

westonruter commented May 22, 2017

jasmussen commented May 23, 2017

klihelp commented May 23, 2017

mkaz commented May 23, 2017

jasmussen commented May 23, 2017

nylen commented May 26, 2017

mkaz commented May 30, 2017

nylen commented May 31, 2017

mkaz commented May 31, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkaz commented Jun 7, 2017

mtias commented Jun 8, 2017

jasmussen commented Jun 9, 2017

StaggerLeee commented Aug 17, 2017

mkaz commented Aug 18, 2017

StaggerLeee commented Aug 18, 2017