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

Update Aztec to use new media pickers (requires Xcode 15) #22060

Merged
merged 9 commits into from
Nov 27, 2023
Merged

Conversation

kean
Copy link
Contributor

@kean kean commented Nov 17, 2023

This is one of the remaining prerequisites for fully decommissioning WPMediaPicker.

Note: this PR requires Xcode 15; please test it locally.

Changes

  • Integrate SiteMediaPickerViewController for selection from your site media
  • Integrate PHPickerViewController for selection from Photos
  • Use UIImagePickerController for camera capture
  • Use PHPickerViewController to replace the embedded picker. It’s available only on iOS 17, which I think is an acceptable trade-off, considering the extremely low number of Aztec users and people who use this option. The iOS 17 adoption has been lagging behind, but by the time this release I expect at least 50% of users to be on iOS 17. I considered implementing a custom picker using Photos.framework and PHAsset, but since the app no longer asks for Photos permissions anywhere else, I don't think it's viable. It also would've required full access to Photos (not "Only Selected")
Screenshot 2023-11-17 at 3 20 00 PM Screenshot 2023-11-17 at 3 20 15 PM

To test:

Prerequisite: In EditorFactory.swift, update EditorFactory instantiateEditor` to always return Aztec.

Embedded picker

Inserting Media

  • Select images, videos from the embedded picker
  • Upload the post and verify that the items were displayed
  • Upload a GIF image and verify that the editor displayed a "GIF" badge

Keyboard Toolbar

  • Open embedded picker
  • Select a couple of assets
  • Verify that the “Insert X” keyboard toolbar button is continuously updated as items are selected
  • Press “Cancel” (little “x”)
  • Verify that the embedded picker is dismissed
  • Open the picker again
  • Verify that the keyboard toolbar button for inserting assets no longer shows “Insert X” (selected was cleared)

iOS 16 and Earlier

  • Verify that inline picker is not displayed on iOS 16 or earlier
  • Verify that you can still toggle between "Text" and "Media" modes

Camera

  • Verify that you can capture images and videos using "Camera"

Photos

  • Verify that the new picker is used PHPickerViewController
  • Verify that images and videos can be selected and added to the post
  • Verify that "Cancel" dismissed the picker

Site Media

Regression Notes

  1. Potential unintended areas of impact: Aztec (Classic Editor)
  2. What I did to test those areas of impact (or what existing automated tests I relied on): manual;
  3. What automated tests I added (or what prevented me from doing so): n/a

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 force-pushed the task/update-aztec branch from fe2145c to 8a8e7d2 Compare November 17, 2023 21:07
@kean kean added this to the Pending milestone Nov 17, 2023
@kean kean requested a review from momo-ozawa November 17, 2023 21:09
@kean
Copy link
Contributor Author

kean commented Nov 17, 2023

FYI, @twstokes.

@kean kean force-pushed the task/update-aztec branch from fcdbc79 to 7b140bb Compare November 17, 2023 21:10
@kean kean changed the title Update Aztec to use new media pickers Update Aztec to use new media pickers (requires Xcode 15) Nov 17, 2023
@kean
Copy link
Contributor Author

kean commented Nov 17, 2023

Hey, @mokagio. What's the timeline for switching to Xcode 15? This PR requires iOS 17 SDK.

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! 1 small thing - I'm not sure if this is an iOS 17 issue, or a simulator issue, but when I launch the Aztec editor the toolbar is partially obscured. I think normally this shouldn't be a problem because the keyboard will be displayed underneath the toolbar, but thought it may be worth mentioning

Screenshot 2023-11-20 at 16 03 24

@twstokes
Copy link
Contributor

FYI, @twstokes.

This is awesome. Thank you!

@kean kean force-pushed the task/update-aztec branch from 7b140bb to 1099db6 Compare November 27, 2023 13:32
@wpmobilebot
Copy link
Contributor

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 Numberpr22060-dc03e76
Version23.7
Bundle IDorg.wordpress.alpha
Commitdc03e76
App Center BuildWPiOS - One-Offs #7964
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

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 Numberpr22060-dc03e76
Version23.7
Bundle IDcom.jetpack.alpha
Commitdc03e76
App Center Buildjetpack-installable-builds #6988
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@kean kean merged commit 45b0375 into trunk Nov 27, 2023
@kean kean deleted the task/update-aztec branch November 27, 2023 15:28
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.

4 participants