-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Conversation
@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 |
@mkaz to be wrapped around the if ( multiple ) {
onSelect( selectedImages.models.map( ( model ) => model.toJSON() ) );
} else {
onSelect( selectedImages.models[ 0 ].toJSON() );
} |
@westonruter yeh, I think that'd be a good spot to add it 👍 |
@mkaz I'll cherry-pick your relevant commit(s) into this branch. |
@westonruter I can help out, see a2d99ad which adds the function to the branch. |
blocks/media-upload-button/index.js
Outdated
return; | ||
} | ||
if ( multiple ) { | ||
onSelect( slimImageObjects( selectedImages.models.map( ( model ) => model.toJSON() ) ) ); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Humm. Build is failing:
I'm going to rebase against |
f5df81e
to
a6aadd2
Compare
Rebased. Former |
multiple: 'add', | ||
editable: false, | ||
|
||
library: wp.media.query( _.defaults( { |
There was a problem hiding this comment.
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.
Rename iOS module name of sample app from gutenberg to GutenbergDemo.
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.