Skip to content

Hide Giphy Picker#10802

Merged
shiki merged 16 commits intodevelopfrom
issue/10801-remove-giphy-from-featured-image
Nov 19, 2019
Merged

Hide Giphy Picker#10802
shiki merged 16 commits intodevelopfrom
issue/10801-remove-giphy-from-featured-image

Conversation

@shiki
Copy link
Copy Markdown
Contributor

@shiki shiki commented Nov 14, 2019 •
edited
Loading

Fixes #10801

Before After
image image

This removes the Giphy option from Actec, Media Library, and Featured Image. The connections to the GiphyPickerActivity have been removed. The Giphy SDK dependency was also removed.

The GiphyPickerActivity and its related classes were kept based on @frosty's advice.

References:

Testing

Please test that:

  1. You can't use Giphy in Aztec
  2. You can't use Giphy in Media Library.
  3. You can't use Giphy in Post Settings → Featured Image.

Reviewing

Only 1 reviewer is needed but anyone can review.

Submitter Checklist

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.
  • If it's feasible, I have added unit tests.

@shiki shiki added this to the 13.7 milestone Nov 14, 2019
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 14, 2019 •
edited
Loading

You can test the changes on this Pull Request by downloading the APK here.

@shiki shiki changed the title Hide Giphy Picker for Featured Image Hide Giphy Picker everywhere Nov 18, 2019
@shiki shiki changed the title Hide Giphy Picker everywhere Hide Giphy Picker Nov 18, 2019
shiki added 15 commits November 18, 2019 13:57
@shiki
Hide Giphy Picker for Featured Image
1b721ed 
We could support it but we’ll have to refactor Giphy Picker to support single-image picking. Right now, it allows picking multiple images and it can be confusing to the user.
@shiki
Remove RequestCodes.GIPHY_PICKER
917cb3d
@shiki
Remove Giphy from GiphyPickerActivity
c24898d
@shiki
Remove Giphy from PhotoPickerFragment
d3ef1c1
@shiki
Remove Giphy from MediaBrowserActivity
af829b8
@shiki
Remove ActivityLauncher.showGiphyPickerForResult
970e270
@shiki
Add comment about Giphy removal
1a0a24b
@shiki
Update RELEASE-NOTES for Giphy removal
8c11840
@shiki
Remove unused R.string.photo_picker_giphy
eb32cac
@shiki
Remove giphy_api_key from gradle.props example
d18413d
@shiki
Remove Giphy SDK dependency
93ef3ea
@shiki
Remove Giphy dependency from GiphyPickerDataSource
392ce25
@shiki
Remove giphy dependency from GiphyPickerDataSourceFactory
5441ae6
@shiki
Remove Giphy depeendency from GiphyMediaViewModel
1f61fab
@shiki
Remove extra line in GiphyPickerDataSource
95d7586
@shiki shiki force-pushed the issue/10801-remove-giphy-from-featured-image branch from abff168 to 95d7586 Compare November 18, 2019 20:57
@shiki shiki marked this pull request as ready for review November 18, 2019 21:58
@jkmassel
Copy link
Copy Markdown
Contributor

jkmassel commented Nov 18, 2019

Howdy folks! I'm freezing 13.7 today, so this is being bumped to 13.8. If you'd like it to go out as part of 13.7, please target release/13.7 for this PR, and DM me once it's merged – I'll be happy to cut a new beta release!

@jkmassel jkmassel modified the milestones: 13.7, 13.8 Nov 18, 2019
planarvoid
planarvoid reviewed Nov 19, 2019
View reviewed changes
WordPress/src/main/java/org/wordpress/android/viewmodel/giphy/GiphyPickerDataSource.kt
@@ -14,7 +11,6 @@ import com.giphy.sdk.core.network.api.GPHApiClient
* [searchQuery] is changed by the user.
*/
class GiphyPickerDataSource(
Copy link
Copy Markdown
Contributor

@planarvoid planarvoid Nov 19, 2019

Choose a reason for hiding this comment

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

I'm wondering if this should be renamed to something like GifPickerDataSource so once we implement a solution with Tenor, it would be obvious to put it here. I guess it's not that important though.

Copy link
Copy Markdown
Contributor Author

@shiki shiki Nov 19, 2019

Choose a reason for hiding this comment

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

That's a good idea. We could probably do it when we switch to a different provider. Git has some quirkiness when files and folders are renamed. It always seemed harder for me to track the history of those. In this case, we probably want to make it easier to see the history of the files and be able to integrate the new provider a bit faster.

planarvoid
planarvoid approved these changes Nov 19, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@planarvoid planarvoid left a comment

Choose a reason for hiding this comment

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

Looks good 👍 Sorry to see your changes go 😉

@shiki
Copy link
Copy Markdown
Contributor Author

shiki commented Nov 19, 2019

Thank you, @planarvoid!

@shiki shiki merged commit 29dfe98 into develop Nov 19, 2019
@shiki shiki deleted the issue/10801-remove-giphy-from-featured-image branch November 19, 2019 14:04
@planarvoid planarvoid mentioned this pull request Mar 17, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wpmobilebot wpmobilebot wpmobilebot left review comments

@planarvoid planarvoid planarvoid approved these changes

@malinajirka malinajirka Awaiting requested review from malinajirka

Assignees

No one assigned

Labels

Media Posting/Editing

Projects

None yet

Milestone

13.8

Development

Successfully merging this pull request may close these issues.

Post Settings: featured image option "Choose from Giphy" is not working

4 participants

@shiki @jkmassel @planarvoid @wpmobilebot