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

Stories/add media picker for empty blocks #14588

Merged
merged 9 commits into from
May 7, 2021

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented May 4, 2021

Fixes Automattic/stories-android#593

See related Stories PR Automattic/stories-android#675

To test:
General case

  1. create a regular Blog Post on Gutenberg mobile
  2. from the block picker, select a Story block to be added
  3. tap on the ADD MEDIA button in the Story block placeholder
  4. observe the Story composer is launched and the media picker is presented.
  5. select some media items and the checkmark, then observe these media items get properly added to the Story.
  6. alternatively to step 5, when the media picker appears, tap on the camera FAB icon to launch the camera mode in ComposerLoopFrameActivity. Capture an image with the camera and observe it gets added as a slide to the Story.

EDIT: I've been touching on some corner cases so I added some more manual test cases here below.

CASE A: first run and cancel (StoryRepository is empty)

  1. Run the app
  2. Create a Gutenberg Post and add an empty Story block
  3. tap on ADD MEDIA, observe the media picker appears.
  4. cancel the action (tap back or the cross on the upper left corner)
  5. observe you land again on the Gutenberg editor.

CASE B: second run and cancel (StoryRepository is empty)

  1. Run the app
  2. Create a Story directly or try editing an existing Story, so the StoryRepository is not empty.
  3. repeat CASE A

CASE C: first run and add media (StoryRepository is empty)

  1. Run the app
  2. Create a Gutenberg Post and add an empty Story block
  3. tap on ADD MEDIA, observe the media picker appears.
  4. select some media items to add
  5. observe you land on the StoryComposer with as many slides as selected
  6. verify images work, videos work, etc.
  7. add some added views and publish your story
  8. verify the story is published correctly
  9. verify you can edit the Story and it loads again in the StoryComposer

CASE D: second run and add media (StoryRepository is empty)

  1. after running CASE C, create a new post and add an empty Story block (or add a new empty Story block to an existing Post)
  2. tap on ADD MEDIA, observe the media picker appears.
  3. select some media items to add
  4. observe you land on the StoryComposer with as many slides as selected
  5. verify images work, videos work, etc.
  6. add some added views and publish your story
  7. verify the story is published correctly
  8. verify you can edit the Story and it loads again in the StoryComposer
  9. edit some, verify your updates, verify it gets saved and updated on the Post.

CASE E: mix photos and video taken with the Stories library's camera functions (cameraX)

  1. create a new post and add an empty Story block
  2. tap on ADD MEDIA, observe the media picker appears.
  3. tap on the camera FAB icon
  4. observe the camera mode is presented
  5. capture an image or a video
  6. observe it gets added as a slide
  7. publish your story and verify it's all as expected.

Regression Notes

  1. Potential unintended areas of impact
    N/A this only affects Stories empty block handling

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N/A

  3. What automated tests I added (or what prevented me from doing so)
    N/A this only changes an extra in an Intent

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@mzorz mzorz added this to the 17.4 milestone May 4, 2021
@mzorz mzorz requested a review from aforcier May 4, 2021 09:19
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 4, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@mzorz
Copy link
Contributor Author

mzorz commented May 4, 2021

Once this PR is tested, we can update the stories library hash which is conflicting with the base branch 👍

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented May 4, 2021

You can test the changes on this Pull Request by downloading the APK here.

@aforcier
Copy link
Contributor

aforcier commented May 7, 2021

@mzorz I merged in develop and updated the hash to the latest stories hash (with Automattic/stories-android#675 merged in). I was retesting when I noticed that the WP media library icon is missing when the media picker is launched from a new story block:

media-picker-from-new

It's there as usual if you then add media from within the story creator:

media-picker-from-story

I'm not sure if we're maybe using the wrong picker, or else something from this PR needs to be adapted for the case of launching from GB: #13242

@mzorz
Copy link
Contributor Author

mzorz commented May 7, 2021

I was retesting when I noticed that the WP media library icon is missing when the media picker is launched from a new story block:

Nice catch @aforcier ! Will take a look and get back here 👍

@mzorz
Copy link
Contributor Author

mzorz commented May 7, 2021

I was retesting when I noticed that the WP media library icon is missing when the media picker is launched from a new story block

This is fixed now in 8bc6e92, the problem was that the parent Activity (ComposeLoopFrameActivity) would take control of the intent before the StoryComposerActivity would have initialized its viewModel through initViewModel(), where we originally were taking care of extracting the SiteModel parameter from the intent.
ComposeLoopFrameActivity would then call the provided media picker but this would not be able to show site's media given the site passed through was still null and hence would hide the Media icon.

Ready for another pass @aforcier 🙏

@aforcier aforcier self-assigned this May 7, 2021
Copy link
Contributor

@aforcier aforcier left a comment

Choose a reason for hiding this comment

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

Working well! :shipit:

@aforcier aforcier merged commit 3383d56 into develop May 7, 2021
@aforcier aforcier deleted the stories/add-media-picker-for-empty-blocks branch May 7, 2021 23:24
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.

Editing empty block: should present the media picker
2 participants