Gallery Widget #120

obenland · 2017-05-02T22:15:16Z

Fixes #62.

See https://core.trac.wordpress.org/ticket/41914

First pass. Fixes #62.

Very basic, needs much more polish. And styles.

timmyc · 2017-05-03T21:02:23Z

Looks like @westonruter already commented but I saw this too:

 ↵Notice: Array to string conversion in /srv/www/wordpress-develop/public_html/src/wp-content/plugins/wp-core-media-widgets/wp-includes/widgets/class-wp-widget-media.php on line 330 ↵Array

Which was resulting in some JS errors in the preview render. I added some logic to ensure we are working against an array in the preview, but agree we should maybe consider returning just the image urls needed in attachments? Not entirely sure the best thing to do.

🤞 that 905edbc also clears up the travis issues.

timmyc · 2017-05-03T21:07:08Z

I disabled the code sniffer in a few lines as it was returning the following error about rand:

wp-includes/widgets/class-wp-widget-media-gallery.php:112:38: error - Detected forbidden query_var "orderby" of "rand". Use vip_get_random_posts() instead. (WordPress.VIP.OrderByRand.orderby_orderby)

Figured that didn't really apply here.

westonruter · 2017-05-03T23:57:22Z

I disabled the code sniffer in a few lines as it was returning the following error about rand

@timmyc feel free to give the WordPress.VIP.OrderByRand.orderby_orderby error a severity of 0 in the underlying phpcs.xml.dist instead.

westonruter · 2017-05-22T19:57:05Z

Cross-reference Gallery block: WordPress/gutenberg#740

First pass. Fixes #62.

Very basic, needs much more polish. And styles.

/** * TODO fix edit button */

timmyc · 2017-08-09T18:42:12Z

@m1tk00 - excellent, I am not getting errors in the preview anymore!! Outside of the linting issues we are having on the branch now - the following flow appears to be broken for me:

Create a new Gallery Widget, assign photos, finish the modal wizard
Click "Edit Gallery" in the widget. Note the current photos are not selected, and the modal state shown is the same as creating a new gallery.

In this scenario, I expected to be shown the same dialog state as seen when clicking the 'pencil' to edit a gallery within the editor:

m1tk00 · 2017-08-09T21:42:13Z

@timmyc yeah thats where I got stuck. I tried passing the data via the selected option but can't get it working

timmyc · 2017-08-14T21:17:48Z

@m1tk00 so looking at how the Gutenberg Gallery block performs the operation: https://github.com/WordPress/gutenberg/blob/master/blocks/media-upload-button/index.js#L124-L141 - they hook into the onOpen event then add in the selected items at that point. Do you want to give that a try or do you want me to work on that here?

m1tk00 · 2017-08-14T23:25:52Z

Ill try it in the morning and let you know :)

…

On Mon, Aug 14, 2017 at 23:17 Timmy Crawford ***@***.***> wrote: @m1tk00 <https://github.com/m1tk00> so looking at how the Gutenberg Gallery block performs the operation: https://github.com/WordPress/gutenberg/blob/master/blocks/media-upload-button/index.js#L124-L141 - they hook into the onOpen event then add in the selected items at that point. Do you want to give that a try or do you want me to work on that here? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#120 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKx1XGG6nrG9_FrPmYUi9UdhZNKfNWSkks5sYLl8gaJpZM4NOwVW> .

m1tk00 · 2017-08-15T10:15:52Z

@timmyc Ive managed to fix the edit button. It was calling a wrong media frame. Just pushed to the repo.

timmyc · 2017-08-15T23:12:52Z

@m1tk00 I'm still having issues with the edit flow - my selections are not being shown when clicking the Edit button.

timmyc · 2017-08-16T00:14:28Z

So the past 3 commits are me trying to get CI to pass :) - each time, there is an array test that seems to go 💥 like the latest:

Parse error: syntax error, unexpected T_STRING in /tmp/wpcs/WordPress/Sniffs/Arrays/CommaAfterArrayItemSniff.php on line 10

Any ideas @westonruter? I tried to re-create the issue locally and could not, then again my phpcs knowledge is limited.

…edia-widgets into add/gallery-widget

joemcgill · 2017-09-12T17:01:47Z

@westonruter last I checked, I don't think gallery settings from the media frame, like columns, etc, are being saved to the widget. I was going to sweep through and test each setting to see what was left.

westonruter · 2017-09-15T22:46:21Z

wp-admin/js/widgets/media-gallery-widget.js

+			previewContainer = control.$el.find( '.media-widget-preview' );
+			previewTemplate = wp.template( 'wp-media-widget-gallery-preview' );
+
+			if ( control.model.get( 'ids' ).length && ! control.model.get( 'attachemnts' ) ) {


! control.model.get( 'attachemnts' ) this looks like it can be eliminated. At least, it is misspelled.

westonruter · 2017-09-15T23:40:40Z

wp-admin/js/widgets/media-gallery-widget.js

+				return attachment.id !== id;
+			} );
+			this.set( {
+				'attachments': JSON.stringify( newAttachments ),


I think references to 'attachments' still needs to be eliminated.

westonruter · 2017-09-15T23:41:27Z

wp-admin/js/widgets/media-gallery-widget.js

+			} );
+			this.set( {
+				'attachments': JSON.stringify( newAttachments ),
+				'ids': _.map( newAttachments, 'id' )


I think this actually should be _.pluck( newAttachments, 'id' ) not map.

westonruter · 2017-09-16T00:34:52Z

I'm working on some improvements to the rendering and syncing of the gallery settings.

…ng and rendering selected attachments

westonruter · 2017-09-16T04:30:07Z

wp-admin/js/widgets/media-gallery-widget.js

+
+			mediaFrameProps = control.mapModelToMediaFrameProps( control.model.toJSON() );
+			if ( mediaFrameProps.size ) {
+				control.displaySettings.set( 'size', mediaFrameProps.size );


This doesn't seem to be used?

westonruter · 2017-09-16T04:30:14Z

wp-admin/js/widgets/media-gallery-widget.js

+
+			mediaFrameProps = control.mapModelToMediaFrameProps( control.model.toJSON() );
+			if ( mediaFrameProps.size ) {
+				control.displaySettings.set( 'size', mediaFrameProps.size );


This doesn't seem to be used?

…since gallery settings are also replaced

…ogic

westonruter · 2017-09-16T04:59:33Z

@joemcgill there's one key thing thing left before I think we can call this ready to merge: customizations.

When you have selected images for a gallery, you then see something like this:

If you try supplying a caption then:

And then it update, there is no changes made because there is no attachments in the model there is nowhere for the customizations to go. Normally in the post editor when you edit a caption it then gets written back to the underlying attachment. However, in the case of media widgets this is blocked entirely with this logic:

// Disable syncing of attachment changes back to server (except for deletions). See <https://core.trac.wordpress.org/ticket/40403>.
defaultSync = wp.media.model.Attachment.prototype.sync;
wp.media.model.Attachment.prototype.sync = function( method ) {
	if ( 'delete' === method ) {
		return defaultSync.apply( this, arguments );
	} else {
		return $.Deferred().rejectWith( this ).promise();
	}
};
mediaFrame.on( 'close', function onClose() {
	wp.media.model.Attachment.prototype.sync = defaultSync;
});

It seems that for the Gallery widget we must remove this logic or else a user will not be able to modify the image attributes.

This passes all of the gallery properties as shortcode atts when rendereing the gallery on the front-end.

joemcgill · 2017-09-16T14:42:03Z

@westonruter I was looking at that syncing logic yesterday and agree with you that in this context, captions should be able to be synced back to the underlying attachment. Your updates to the syncing of props between the widget control and the media frame seem to fix the issues I had noticed but the gallery attributes weren't being used to render the gallery shortcode on the front end, so I pushed another quick commit to resolve that issue.

Will do some more testing, but other than the caption issue, things are looking good from my end. Just a note that captions are a bit tricky because some themes disregard captions when they render galleries.

…can be modified

westonruter · 2017-09-16T23:32:42Z

@joemcgill excellent, thanks. Once you merge this PR then we'll release a new version of the feature plugin to get out to testers for next week prior to core merge.

joemcgill · 2017-09-19T03:49:27Z

@westonruter Got a chance to this again after e5ee972, and captions updates are now saving back to the model correctly. Going to go ahead and merge so we can get this into more hands. Nice work 🎉

westonruter · 2017-09-19T06:20:48Z

Great work everyone!

westonruter · 2017-09-19T06:21:32Z

The plugin auto-deployed to WordPress.org so it is now available for all to test: https://wordpress.org/plugins/wp-core-media-widgets/

obenland and others added 3 commits May 2, 2017 15:14

[WIP] Gallery Widget

634f3cd

First pass. Fixes #62.

Add preview for galleries

f8e7949

Very basic, needs much more polish. And styles.

Fix travis issues, prevent JS error.

905edbc

westonruter mentioned this pull request May 22, 2017

Add Gallery Block WordPress/gutenberg#740

Merged

This was referenced Jul 9, 2017

Gallery: add missing options to inspector WordPress/gutenberg#1448

Closed

Open the gallery from placeholder in the initial creation state WordPress/gutenberg#1820

Merged

obenland and others added 4 commits August 2, 2017 10:06

[WIP] Gallery Widget

77463cd

First pass. Fixes #62.

Add preview for galleries

46fe3b4

Very basic, needs much more polish. And styles.

Fix travis issues, prevent JS error.

75a466c

Rebase and get things somewhat operational again.

88a5d0b

timmyc force-pushed the add/gallery-widget branch from 905edbc to 88a5d0b Compare August 2, 2017 17:16

Fix preview for gallery

984e68f

/** * TODO fix edit button */

Fix linting errors.

9aae63b

Fix edit gallery button.

053147f

timmyc added 3 commits August 15, 2017 16:51

Not sure how to fix travis failing, trying this.

a063060

try that again

69e7a8f

Remove another rule to try and get CI to pass.

ac13f7c

m1tk00-devv added 2 commits August 16, 2017 11:27

Fix edit gallery button

41b6ea9

Merge branch 'add/gallery-widget' of https://github.com/xwp/wp-core-m…

0203390

…edia-widgets into add/gallery-widget

westonruter reviewed Sep 15, 2017

View reviewed changes

Allow gallery preview clicking to be shortcut for edit gallery button

a4dacce

westonruter added 3 commits September 15, 2017 19:53

Persist random order setting from gallery; refactor method for fetchi…

fd47023

…ng and rendering selected attachments

Fix rendering on change and reset

ece64e9

Add missing replace_media label

be35f36

westonruter reviewed Sep 16, 2017

View reviewed changes

westonruter added 6 commits September 15, 2017 21:32

Ensure all gallery settings get synced to widget

e1429f0

Update button label to 'Replace Gallery' instead of 'Replace Images' …

94d6ab2

…since gallery settings are also replaced

Remove obsolete qunit test

f2c6a49

Add handleAttachmentDestroy to factor out former removeAttachmentId l…

4e343ef

…ogic

Ensure r41248 from core is applied to defaultSync logic

70480f5

Remove obsolete access phpDoc tags

daa0039

joemcgill added 2 commits September 16, 2017 09:25

Use all gallery shortcode properties when rendering shortcode.

e82a420

This passes all of the gallery properties as shortcode atts when rendereing the gallery on the front-end.

Whitespace fix

775f498

Remove disabling of syncing attachment changes to server so captions …

e5ee972

…can be modified

westonruter approved these changes Sep 16, 2017

View reviewed changes

joemcgill merged commit cec2c4f into master Sep 19, 2017

westonruter deleted the add/gallery-widget branch September 19, 2017 06:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gallery Widget #120

Gallery Widget #120

obenland commented May 2, 2017 •

edited by westonruter

Loading

timmyc commented May 3, 2017

timmyc commented May 3, 2017 •

edited

Loading

westonruter commented May 3, 2017

westonruter commented May 22, 2017

timmyc commented Aug 9, 2017

m1tk00 commented Aug 9, 2017

timmyc commented Aug 14, 2017

m1tk00 commented Aug 14, 2017 via email

m1tk00 commented Aug 15, 2017

timmyc commented Aug 15, 2017

timmyc commented Aug 16, 2017

joemcgill commented Sep 12, 2017

westonruter Sep 15, 2017

westonruter Sep 15, 2017

westonruter Sep 15, 2017

westonruter commented Sep 16, 2017

westonruter Sep 16, 2017

westonruter Sep 16, 2017

westonruter commented Sep 16, 2017

joemcgill commented Sep 16, 2017

westonruter commented Sep 16, 2017

joemcgill commented Sep 19, 2017

westonruter commented Sep 19, 2017

westonruter commented Sep 19, 2017

Gallery Widget #120

Gallery Widget #120

Conversation

obenland commented May 2, 2017 • edited by westonruter Loading

timmyc commented May 3, 2017

timmyc commented May 3, 2017 • edited Loading

westonruter commented May 3, 2017

westonruter commented May 22, 2017

timmyc commented Aug 9, 2017

m1tk00 commented Aug 9, 2017

timmyc commented Aug 14, 2017

m1tk00 commented Aug 14, 2017 via email

m1tk00 commented Aug 15, 2017

timmyc commented Aug 15, 2017

timmyc commented Aug 16, 2017

joemcgill commented Sep 12, 2017

westonruter Sep 15, 2017

Choose a reason for hiding this comment

westonruter Sep 15, 2017

Choose a reason for hiding this comment

westonruter Sep 15, 2017

Choose a reason for hiding this comment

westonruter commented Sep 16, 2017

westonruter Sep 16, 2017

Choose a reason for hiding this comment

westonruter Sep 16, 2017

Choose a reason for hiding this comment

westonruter commented Sep 16, 2017

joemcgill commented Sep 16, 2017

westonruter commented Sep 16, 2017

joemcgill commented Sep 19, 2017

westonruter commented Sep 19, 2017

westonruter commented Sep 19, 2017

obenland commented May 2, 2017 •

edited by westonruter

Loading

timmyc commented May 3, 2017 •

edited

Loading