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

Integrate PHPickerViewController in Story Editor #22059

Merged
merged 6 commits into from
Nov 20, 2023
Merged

Conversation

kean
Copy link
Contributor

@kean kean commented Nov 17, 2023

Changes:

  • Integrate PHPickerViewController in Story Editor
  • No permission prompts are needed anymore. The only downside is that the "Add Photo" button will no longer be able to display a tiny preview of your latest photo from the gallery.
  • Removes "Site Media" as a source (nobody using it; not worth re-implementing)
  • Set standard maximum export resolution for images to 2048x2048 regardless of the device from which the story is created

There should be no other functional changes.
The goal is to decommission WPMediaPicker in this release.

To test:

🟡 This feature requires access to the camera and doesn’t seem to work in the simulator, even when selecting photos from the local library. I haven’t looked into it. Please test on the physical device.

To create a story, tap the "Create" button on the Home Screen and select "Story Post". Alternatively, add a "Story" block to an existing post.

  • Verify that videos and images can be selected
  • Verify that “Cancel” works
  • Verify that the Editor no longer requires the permissions or asks for the permissions to access Photos (best tested using a clean build)
  • Verify that adding photos works
  • Verify that uploading both portrait and horizontal video works
  • Verify that video longer than 5 minutes can’t be uploaded to free blogs
  • Verify that errors are shown (hardcode in unc picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult])
  • Verify that the story can be uploaded

Regression Notes

  1. Potential unintended areas of impact

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

  3. What automated tests I added (or what prevented me from doing so)

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • 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.

UI Changes testing checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@kean kean added the Media label Nov 17, 2023
@kean kean added this to the Pending milestone Nov 17, 2023
@kean kean requested a review from momo-ozawa November 17, 2023 12:42
@kean kean modified the milestones: Pending, 23.8 Nov 17, 2023
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 17, 2023

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr22059-fb0f2ef
Version23.7
Bundle IDorg.wordpress.alpha
Commitfb0f2ef
App Center BuildWPiOS - One-Offs #7885
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 17, 2023

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr22059-fb0f2ef
Version23.7
Bundle IDcom.jetpack.alpha
Commitfb0f2ef
App Center Buildjetpack-installable-builds #6909
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

Copy link
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

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

Works as described!

@momo-ozawa
Copy link
Contributor

The first time you access the stories editor on a fresh install, you see the above. I thought the button title was a bit strange. This isn't within the scope of this PR (in fact, it's defined in Kanvas), but just noting it down

@kean kean enabled auto-merge November 20, 2023 14:49
@peril-wordpress-mobile
Copy link

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS

@wpmobilebot
Copy link
Contributor

1 Warning
⚠️ This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

@kean kean merged commit 701a249 into trunk Nov 20, 2023
@kean kean deleted the task/update-story-editor branch November 20, 2023 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants