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.

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

Open the gallery from placeholder in the initial creation state #1820

Merged
merged 4 commits into from
Jul 21, 2017

Conversation

westonruter
Copy link
Member

Adds the initial multi-select mode without having to shift-click to select multiple images, as well as the “interstitial” state to re-order the images as desired. This adds functional parity with the classic post editor.

Props @obenland for his work on Gallery widget which was adapted into this PR: xwp/wp-core-media-widgets#120

Fixes #1401.

@westonruter westonruter requested a review from mtias July 9, 2017 06:44
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 17.752% when pulling e890db3 on add/create-gallery-state into 4564ee0 on master.

@mkaz
Copy link
Member

mkaz commented Jul 19, 2017

@westonruter - tested out and reviewed, this one looks good and probably better since it integrates with MediaUploadButton, so we should use this instead of #1940

One thing you might want to add is something along the lines of this slimImageObjects function which attempts to reduce how data getting saved as attributes

cc: @youknowriad

@westonruter
Copy link
Member Author

@mkaz to be wrapped around the toJSON calls in the following?

		if ( multiple ) {
			onSelect( selectedImages.models.map( ( model ) => model.toJSON() ) );
		} else {
			onSelect( selectedImages.models[ 0 ].toJSON() );
		}

@mkaz
Copy link
Member

mkaz commented Jul 19, 2017

@westonruter yeh, I think that'd be a good spot to add it 👍

@westonruter
Copy link
Member Author

@mkaz I'll cherry-pick your relevant commit(s) into this branch.

@mkaz
Copy link
Member

mkaz commented Jul 20, 2017

@westonruter I can help out, see a2d99ad which adds the function to the branch.

return;
}
if ( multiple ) {
onSelect( slimImageObjects( selectedImages.models.map( ( model ) => model.toJSON() ) ) );
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of having both slimImageObjects and slimImageObjects, how about only having the former? If you just have slimImageObject then we can just do:

if ( multiple ) {
	onSelect( selectedImages.models.map( ( model ) => slimImageObject ( model.toJSON() ) ) );
} else {
	onSelect( slimImageObject( selectedImages.models[ 0 ].toJSON() ) );
}

I guess I just don't see how having a separate slimImageObjects function is necessary.

Copy link
Member

Choose a reason for hiding this comment

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

heh, that is better - I think because I started with the first method, I kept it around.

@westonruter
Copy link
Member Author

Humm. Build is failing:

phpenv global 5.2
rbenv: version `5.2' not installed

I'm going to rebase against master and see if that fixes it, since I know there were some Travis changes in the past couple days.

@westonruter
Copy link
Member Author

Rebased. Former HEAD was f5df81e4.

multiple: 'add',
editable: false,

library: wp.media.query( _.defaults( {
Copy link
Member

Choose a reason for hiding this comment

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

This assumes a dependency on Underscore which we don't explicitly define. It's only incidentally not causing catastrophic failure.

Tug pushed a commit that referenced this pull request Feb 28, 2020
Rename iOS module name of sample app from gutenberg to GutenbergDemo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants