Skip to content

Conversation

@kean
Copy link
Contributor

@kean kean commented Nov 27, 2023

Removes WPMediaPicker dependency

To test:

  • Smoke test Gutenberg: verify you can add images from Photos and Site Media
  • Smoke test Site Media: verify that you can open video previews
  • Smoke test Site Icon Picker: verify that picking from Site Media works

Regression Notes

  1. Potential unintended areas of impact: Media
  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)

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Nov 27, 2023

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

Generated by 🚫 dangerJS

@kean kean requested a review from momo-ozawa November 27, 2023 21:27
@kean kean added this to the 23.9 milestone Nov 27, 2023
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 27, 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 Numberpr22103-f51be52
Version23.7
Bundle IDorg.wordpress.alpha
Commitf51be52
App Center BuildWPiOS - One-Offs #7982
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 27, 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 Numberpr22103-f51be52
Version23.7
Bundle IDcom.jetpack.alpha
Commitf51be52
App Center Buildjetpack-installable-builds #7006
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 w a few comments

dataSourceType: .mediaLibrary,
allowMultipleSelection: allowMultipleSelection,
callback: { [weak self] assets in
mediaPickerHelper.presentSiteMediaPickere(filter: filter, allowMultipleSelection: allowMultipleSelection) { [weak self] assets in
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
mediaPickerHelper.presentSiteMediaPickere(filter: filter, allowMultipleSelection: allowMultipleSelection) { [weak self] assets in
mediaPickerHelper.presentSiteMediaPicker(filter: filter, allowMultipleSelection: allowMultipleSelection) { [weak self] assets in

Copy link
Contributor Author

@kean kean Nov 28, 2023

Choose a reason for hiding this comment

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

Fixed 😓

* [**] Add media fitlers to the Site Media screen [#22096]
* [*] The "aspect ratio" mode on the Site Media screen is now also available on iPhone via the new title menu [#22096]
* [**] Update the classic editor to use the new Photos and Site Media pickers [#22060]
* [**] [internal] Remove WPMediaPicker dependency [#22103]
Copy link
Contributor

Choose a reason for hiding this comment

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

wdyt of combining this with the item below this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the entry:

* [*] [internal] Rework Tenor (Free GIF) and Stock Photos (Free Photos) pickers [#22066, #22074]

The goal was to highlight the fact that Tenor and Stock photos changed. So it's more than just about removing WPMediaPicker, and it needs some testing.

@kean kean enabled auto-merge November 28, 2023 14:39
@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 9f6925e into trunk Nov 28, 2023
@kean kean deleted the task/remove-wpmideapicker branch November 28, 2023 15:30
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