Skip to content

Conversation

@mkevins
Copy link
Contributor

@mkevins mkevins commented Nov 27, 2019

This PR aims to consolidate various media selection handling by refactoring methods that utilize both singular and plural paradigms to only use plural methods where possible.

We should preserve the behavior that occurs when multiple images are selected via the media library intent (multiple selection is possible for the image block even though the allowMultipleSelection flag is false).

Related WordPress-Android PR: wordpress-mobile/WordPress-Android#10857

To test:


Image block:

When multiple items are selected via the media library in the image block, the additional items are appended to the editor as new image blocks.


Gallery block:

Related PR: wordpress-mobile/WordPress-Android#10703

Checkout related Gallery PR and build with source.

  • In a post, add a gallery block
  • Tap "ADD MEDIA"
  • Tap "WordPress Media Library"
  • Select multiple images

Expected behavior:

The selected images should be added to the gallery, and not added as new image blocks.


Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@mkevins mkevins marked this pull request as ready for review November 27, 2019 12:33
@mkevins mkevins requested a review from dratwas November 27, 2019 12:46
Copy link
Contributor

@malinajirka malinajirka left a comment

Choose a reason for hiding this comment

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

Thanks @mkevins! LGTM - I'll let @dratwas do the final review + merge 🙇 .

@mkevins mkevins requested a review from dratwas November 29, 2019 01:50
@mkevins
Copy link
Contributor Author

mkevins commented Nov 29, 2019

@malinajirka thanks for pairing / collaborating on this 👍

@dratwas 👋 thanks for reviewing. I've set the mAppendsMultipleSelectedToSiblingBlocks flag explicitly to false in the other media selection paths. I've also left the set-to-false assignment after the for each loop to guard, in case a future media selection intent is added without the explicit assignment (so the default will always be to respect the allowMultipleSelection flag). This is ready for another pass.

@mkevins
Copy link
Contributor Author

mkevins commented Nov 29, 2019

I had to make one additional change to make the enum - mimeType truncation work (the comparison / truncation should be all lowercase).
I tested with Media Library (selecting single and multiple images), and also with uploading from device for the image block. For the uploading flow, I also tested leaving the editor while the upload is in progress, and opening the post after the upload is finished.

@mkevins mkevins merged commit 42d44a5 into develop Dec 3, 2019
@dcalhoun dcalhoun deleted the fix/multiple-media-selection-handling branch August 28, 2023 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants