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

End of Year: Story snapshot and share #505

Merged
merged 13 commits into from
Nov 1, 2022
Merged

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Oct 28, 2022

📘 Project: #410

iOS PR: Automattic/pocket-casts-ios#404

Description

This PR

  • Generates an image for the latest story and passes it to the share intent.

    The image generation uses Android's view.drawToBitmap() function. Selected story's sharable composable is converted to ComposeView and laid out into AndroidView otherwise an exception is thrown: java.lang.IllegalStateException: View needs to be laid out before calling drawToBitmap()

    It only captures the visible screen.

    The share button click action invokes a callback that generates the bitmap, saves it, and opens it in share view.

  • Includes two other changes:

Testing Instructions

  1. Set END_OF_YEAR_ENABLED feature flag to true in base.gradle
  2. Run the app
  3. When the prompt appears, tap "View My 2022"
  4. Tap "Share"
  5. Tap "Edit"
  6. ✅ Make sure the generated image is correct
  7. Go back to the app, and share the story again
  8. Select Instagram or Facebook (story)
  9. ✅ Check that the image appears correctly in those apps

Screenshots or Screencast

Share View Screenshot

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • Any jetpack compose components I added or changed are covered by compose previews

I have tested any UI changes...

  • with different themes
  • with a landscape orientation
  • with the device set to have a large display and font size
  • for accessibility with TalkBack

@ashiagr ashiagr requested a review from a team as a code owner October 28, 2022 13:14
@ashiagr ashiagr marked this pull request as draft October 28, 2022 13:15
@ashiagr ashiagr mentioned this pull request Oct 28, 2022
35 tasks
Use stateIn on the resulting flow returned by combine
https://rb.gy/ew2iqy
@ashiagr ashiagr force-pushed the task/410-story-snapshot branch from 6fff28a to 94276c7 Compare October 31, 2022 06:33
If true - StoryView is a child of StorySwitcher, all inputs to interactive elements of StoryView are consumed by the elements and other inputs are passed to the parent.

If false - StorySwitcher is laid over StoryView and it takes all inputs.
Temporary changes to fit layout to screen size
intent.type = "image/png"
val uri = FileUtil.createUriWithReadPermissions(file, intent, requireContext())
intent.putExtra(Intent.EXTRA_STREAM, uri)
shareLauncher.launch(Intent.createChooser(intent, context.getString(LR.string.end_of_year_share_via)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This launches share intent with an edit button. On clicking "edit" from the chooser, an exception is seen in logcat:

Writing exception to parcel
java.lang.SecurityException: Permission Denial: writing androidx.core.content.FileProvider uri content://<path > requires the provider be exported, or grantUriPermission()

I tried to grant FLAG_GRANT_WRITE_URI_PERMISSION locally but it didn't help. Strangely, testing on an emulator shows logs even for reading the file. Also see related SO discussions link 1, link2.

Sharing and editing work without problems. I still have to see why these logs are shown. Let me know if you have any suggestions. Another option is to not show the edit button, the share dialog will look this way then:

items(story.podcasts.size) { index ->
PodcastItem(
podcast = story.podcasts[index],
iconSize = getIconSize(screenHeight, textFieldHeight, context),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an adjustment for the icon size for the "fake story view" so that the entire list is visible on the screen for taking a snapshot.

Note that iOS locks the screen to portrait and captures only the visible screen. Unlike Android, they don't seem to have split screen feature on mobile which can reduce the screen height considerably. They also convert the dialog to a small modal on iPad. This is not yet done on Android.

@ashiagr ashiagr marked this pull request as ready for review October 31, 2022 12:02
Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

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

This looks good and is working well. I left a few comments, but they're mostly related to code style and I don't consider any of them blockers, so I'll go ahead and approve.

There were a couple of odd things I ran across in testing this...

1. Sudden speed up in Stories

Exactly one time while testing this, I found that after I returned from Sharing a story that the Stories started to run extremely fast (If I tapped to go back to the first story, the little slider for that story would complete in well under 1 second).

I have not been able to reproduce this, so it may be nothing, but I wanted to mention it in case you see it too at some point.

2. It is possible to open the full screen player behind the "Your Year in Podcasts" bottom sheet

When the "Your Year in Podcasts" bottom sheet shows, it is possible to swipe up to open the full screen player. This certainly doesn't need to be addressed in this PR.

device-2022-10-31-132159.mp4

@ashiagr
Copy link
Contributor Author

ashiagr commented Nov 1, 2022

Thank you for all the great suggestions, @mchowning! I've addressed your comments except the following:

  1. Sudden speed up in Stories
  2. It is possible to open the full screen player behind the "Your Year in Podcasts" bottom sheet

I'll add them to the main task and address them in future PRs.

@ashiagr ashiagr merged commit bea60f1 into main Nov 1, 2022
@ashiagr ashiagr deleted the task/410-story-snapshot branch November 1, 2022 06:02
@ashiagr ashiagr added this to the 7.27 ❄️ milestone Nov 24, 2022
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.

2 participants