Tenor integration #13949

ScoutHarris · 2020-04-21T00:47:39Z

This PR is to merge the Tenor feature branch into develop. It encompasses @ngoctr 's PRs for the tasks marked as complete on #13803 .

Accessing the Free GIF Library from any of the following paths presents the same Tenor picker, and should all look/behave the same:

To test:

Media Library:

Go to Site Details > Media.
Select + on the nav bar.
Verify Free GIF Library is an option. Select it.
Search for a GIF.
Select GIFs.
Verify they are added to the Media Library.

Block Editor:

Edit a post.
Ensure you're using the block editor.
Add an Image or Gallery block.
Select Add Image (Image block) / Add Media (Gallery block).
Verify Free GIF Library is an option. Select it.
Search for a GIF.
Select GIFs.
Verify they are added to the block.

NOTE: there is a known issue where Previewing a post before media finishes uploading breaks the media. This applies to any media type, in both editors.

Classic Editor:

Edit a post.
Ensure you're using the classic editor.
Tap the ellipsis button on the media toolbar.
Verify Free GIF Library is an option. Select it.
Search for a GIF.
Select GIFs.
Verify they are added to the post.

PR submission checklist:

I have considered adding unit tests where possible.
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.

…ture-flag

Add Tenor feature flag

Split unit tests into smaller ones

Tenor API credentials support

Implement Tenor client api

…icker

…review

frosty

Code looks good to me, just a few comments there and then the functional issues I mentioned before.

I've also seen the Couldn't decode API response from Tenor error logged out a few times while testing this, but I'm not sure why yet.

frosty · 2020-04-24T11:19:13Z

WordPress/Classes/Utility/Analytics/WPAppAnalytics+Media.swift

        return origin.eventForMediaType(mediaType)
    }

+    // Old tracking events via WPShared
+    // Ref: https://iosp2.wordpress.com/2020/03/16/adding-tracks-forget-wordpress-shared/


Probably shouldn't include a p2 reference here

Two more of these below too

Done thanks @frosty

frosty · 2020-04-24T11:22:53Z

WordPress/Classes/Utility/Analytics/WPAppAnalytics+Media.swift

-    func eventForMediaType(_ mediaType: MediaType) -> WPAnalyticsStat? {
+    // All new media tracking events will be added into WPAnalyticsEvent
+    // Ref: https://iosp2.wordpress.com/2020/03/16/adding-tracks-forget-wordpress-shared/
+    func eventForMediaType(_ mediaType: MediaType) -> WPAnalyticsEvent? {


I'm not entirely sure why these definitions need to be split out from the existing definitions below?

Hey @frosty because the old tracking method uses WPAnalyticsStat from WPShared pod while the new method uses a new enum WPAnalyticsEvent

frosty · 2020-04-24T11:23:13Z

WordPress/Classes/Utility/Analytics/WPAppAnalytics+Media.swift

@@ -159,4 +186,5 @@ enum MediaSource {
    case camera
    case giphy


Note for a future PR: we should remove all the existing Giphy references

There is already a task for this #13803 ;-)

Great, thanks!

frosty · 2020-04-24T13:15:54Z

WordPress/Classes/ViewRelated/Media/StockPhotos/AztecMediaPickingCoordinator.swift

@@ -64,6 +76,10 @@ final class AztecMediaPickingCoordinator {
        giphy.presentPicker(origin: origin, blog: blog)
    }

+    private func showTenor(origin: UIViewController, blog: Blog) {
+        tenor.presentPicker(origin: origin, blog: blog)


The media library and Gutenberg both create a new Tenor picker each time one needs to be presented. Should we do the same here?

I assume it should be a YES answer. I believe the Gutenberg editor is the future of editor (69% uses have used it) so o I will make a change for this to make it consistent to Aztec/Media.

frosty · 2020-04-24T13:16:22Z

WordPress/Classes/ViewRelated/Media/Tenor/NoResultsTenorConfiguration.swift

+    static func configureAsIntro(_ viewController: NoResultsViewController) {
+        viewController.configure(title: .tenorPlaceholderTitle,
+                                 image: Constants.imageName,
+                                 subtitleImage: "tenor-attribution")


This could probably go in the Constants enum too.

Done. Thanks @frosty

frosty · 2020-04-24T13:49:12Z

WordPress/Classes/ViewRelated/Media/Tenor/TenorMediaGroup.swift

+    }
+
+    func numberOfAssets(of mediaType: WPMediaType, completionHandler: WPMediaCountBlock? = nil) -> Int {
+        return 10


Do we know why this is set to 10?

The TenorMediaGroup is created just to make the TenorDataSource conformed to the WPMediaCollectionDataSource protocol. Since we haven't supported media groups (e.g group by categories) in all media sources (StockPhotos, Giphy, Tenor). So 10 or any doesn't matter. StockPhotos uses 10, Giphy uses 10, Tenor (is a Giphy clone), has 10 too. :d

ngoctr · 2020-04-24T23:48:43Z

@frosty thanks for the review. I will try to address your comments as much as I can. Some of them were not entirely belong to the Tenor Picker itself, it's part of the app's existing and (inconsistent) behaviour so they should be tracked separately ;-)

ngoctr · 2020-04-25T01:20:57Z

There's an image with cell or overlay reuse in the table. If you already performed a search, and then you perform a new one, you first see the gif results from the original search and then they're gradually replaced with the new results. Video here: https://cloudup.com/cC823L9QA3B

To support animating GIF, the implementation of Tenor Picker injected an Overlay view for WPMediaCollectionViewCell and with the current design of the WPMediaPicker it's not possible to clear/reset the overlay view (from where the overlay is injected (WPiOS app)) when preparing to reuse a cell. So we will need to consider a bit more improvement on the WPMediaPicker itself in order to handle this situation.

Improvements for Tenor picker to address comments in #13949

frosty

I think let's get this merged, but keep it feature flagged and ensure we have any remaining items captured in the master issue (I just added all of my issues there).

ScoutHarris · 2020-05-12T19:27:37Z

Thanks @frosty !

ngoctr and others added 30 commits April 6, 2020 14:38

Added Tenor feature flag

e857e35

Merge branch 'feature/13803-tenor_integration' into feature/tenor-fea…

4a33eb3

…ture-flag

Merge pull request #13829 from ngoctr/feature/tenor-feature-flag

7bae58f

Add Tenor feature flag

Merge branch 'develop' into feature/13803-tenor_integration

8ec416b

Added Tenor API Key support in gencredentials.rb

6b19638

Added Tenor Client API

30a06f9

Added more unit tests for Tenor API response

37ca6ba

Added nanoGIF into TenorGIFCollection to use as thumbnail

b0e7160

Split unit tests into smaller ones

Removed file header generated by Xcode

bb32ab4

Merge branch 'develop' into feature/13803-tenor_integration

aa199f2

Print error if Tenor key not being configure

5f50901

Fixed a typo in code comment

1e56339

Removed unnecessary properties that we don't need them

f0fbfd1

Use DDLogError instead of assertionFailture

f5dbe08

Added comment and explicitly add public modifier for search method

b46ccde

Merge pull request #13842 from ngoctr/13803/tenor-api-credentials

53fd5fa

Tenor API credentials support

Merge pull request #13843 from ngoctr/13803/tenor-client-api

ae01693

Implement Tenor client api

Merge branch 'develop' into feature/13803-tenor_integration

29a5a23

Merge branch 'develop' into feature/13803-tenor_integration

8dbadf5

Implement Tenor media picker

0773f8d

Added tenor-attribution image

8e47b28

Added unit tests for TenorPageable

579fa89

Fixed a typo on variable name

95c6f4f

Used nano GIF size from Tenor for a faster thumbnail loading in the P…

b9c58d2

…icker

Used largest GIF image size from Tenor for user's media library and p…

e2b30ea

…review

Removed commented code

5a243d2

Fixed a build issue

c4b336c

Moved mock data into json resource files

eecacb8

Added TenorResultsPage tests

fe9884b

Added Tenor media exporter

d81e424

Merge branch 'develop' into feature/13803-tenor_integration

6e31736

frosty reviewed Apr 24, 2020

View reviewed changes

ngoctr added 5 commits April 25, 2020 10:11

Removed reference to P2

125be33

Added contentFilter param to Tenor Client API

5258dec

Made subtitle image name a constant

2e90a13

Created a new Tenor picker each time being invoked in Aztec Editor

1a96c43

Correctly releasing the Tenor picker instance

ae4e567

ngoctr mentioned this pull request Apr 25, 2020

Improvements for Tenor picker to address comments in #13949 #14001

Merged

3 tasks

ngoctr and others added 3 commits April 25, 2020 11:33

Removed unnecessary guards

1d9fdf6

Merge branch 'develop' into feature/13803-tenor_integration

65e3a9b

Merge branch 'develop' into feature/13803-tenor_integration

73494fd

ScoutHarris removed this from the 14.8 milestone Apr 30, 2020

ScoutHarris added the [Status] DO NOT MERGE label Apr 30, 2020

ScoutHarris and others added 5 commits May 4, 2020 11:26

Merge branch 'develop' into feature/13803-tenor_integration

1bcf08a

Merge branch 'develop' into feature/13803-tenor_integration

71573b7

Merge pull request #14001 from ngoctr/13803-tenor-master-branch

e4139be

Improvements for Tenor picker to address comments in #13949

TenorClient: Added comment to clarify content filter ratings.

25c6d0a

Merge branch 'develop' into feature/13803-tenor_integration

a8aab30

frosty approved these changes May 11, 2020

View reviewed changes

Merge branch 'develop' into feature/13803-tenor_integration

d35ce72

ScoutHarris removed the [Status] DO NOT MERGE label May 12, 2020

ScoutHarris added this to the 14.9 milestone May 12, 2020

ScoutHarris merged commit bdf30ae into develop May 12, 2020

ScoutHarris deleted the feature/13803-tenor_integration branch May 12, 2020 20:02

ScoutHarris restored the feature/13803-tenor_integration branch May 13, 2020 01:26

ScoutHarris deleted the feature/13803-tenor_integration branch June 1, 2020 17:57

ScoutHarris mentioned this pull request Jul 23, 2020

Remove Tenor feature flag. #14513

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tenor integration #13949

Tenor integration #13949

ScoutHarris commented Apr 21, 2020

frosty left a comment

frosty Apr 24, 2020

frosty Apr 24, 2020

ngoctr Apr 25, 2020

frosty Apr 24, 2020

ngoctr Apr 24, 2020

frosty Apr 24, 2020

ngoctr Apr 24, 2020

frosty Apr 27, 2020

frosty Apr 24, 2020

ngoctr Apr 25, 2020

frosty Apr 24, 2020

ngoctr Apr 25, 2020

frosty Apr 24, 2020

ngoctr Apr 24, 2020

ngoctr commented Apr 24, 2020 •

edited

Loading

ngoctr commented Apr 25, 2020 •

edited

Loading

frosty left a comment

ScoutHarris commented May 12, 2020

Tenor integration #13949

Tenor integration #13949

Conversation

ScoutHarris commented Apr 21, 2020

frosty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngoctr commented Apr 24, 2020 • edited Loading

ngoctr commented Apr 25, 2020 • edited Loading

frosty left a comment

Choose a reason for hiding this comment

ScoutHarris commented May 12, 2020

ngoctr commented Apr 24, 2020 •

edited

Loading

ngoctr commented Apr 25, 2020 •

edited

Loading