Skip to content

Replace UI and internal Giphy references#3

Merged
ThomazFB merged 7 commits intoissue/11456-add-tenor-supportfrom
feature/adjust-tenor-strings
Apr 1, 2020
Merged

Replace UI and internal Giphy references#3
ThomazFB merged 7 commits intoissue/11456-add-tenor-supportfrom
feature/adjust-tenor-strings

Conversation

@ThomazFB
Copy link
Copy Markdown
Owner

@ThomazFB ThomazFB commented Apr 1, 2020

Summary

This Pull Request brings renaming for all Giphy references inside package, classes, documentation and UI strings. Giving sequence to the issue 11456.

Here's the change log:

  • Every UI string with Giphy information was replaced with Tenor.

  • The giphy packages inside ui and viewmodel were both renamed to only gif.

  • All classes with the Giphy naming convention were renamed to only Gif to better fit the agnostic gif provider implementation strategy.

  • All comments and documentation with references to Giphy or the Giphy SDK were readapted to better fit the GifProvider structure.

  • Removed the Giphy SDK reference at maven repository in build.gradle

Testing

Before testing

  1. Create a Tenor API key and paste it associated with the wp.tenor.api_key as the value inside your gradle.properties
  2. When testing verify that all UI strings correctly reference Tenor as the gif provider.

Testing from the Post Editor

  1. Click to create a new post
  2. Click on the overflow button at the top right end of the screen
  3. select the Switch to classic editor
  4. Click on the plus (+) button at the bottom left end of the screen
  5. Select the Choose from Tenor option
  6. Type in the search bar to query GIFs
  7. Once inside the Gif Picker, make sure that selection, preview and add are all working as expected

Testing from the Media Browser

  1. Go to the Media Browser
  2. Click on the Add button at the top right. Select the Choose from Tenor option
  3. Type in the search bar to query GIFs
  4. Once inside the Gif Picker, make sure that selection, preview and add are all working as expected

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.

@ThomazFB ThomazFB changed the title Feature/adjust tenor strings Replace UI and internal Giphy references Apr 1, 2020
Copy link
Copy Markdown

@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.

Overall looks pretty great! I have one comment about the translations but that should be fairly simple to change. Thanks for all the changes 👍

<string name="stats_no_data_for_period">Keine Daten für diesen Zeitraum</string>
<string name="preference_strip_image_location">Positionsangabe von Medien entfernen</string>
<string name="stats_cannot_be_started">Wir können die Statistiken momentan nicht öffnen. Bitte versuche es später erneut</string>
<string name="giphy_powered_by_giphy">Bereitgestellt von GIPHY</string>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Maybe I didn't mention this explicitly. We shouldn't change any of the translation files. It should all go through the translation service. You should just change the names of the string items in the basic strings.xml (and you can also delete the translations of the original strings). I think (I'm not sure here) they'll get overriden by the service result anyway.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I just deleted all translation strings as suggested and left the renaming and the Tenor replacement only at the original strings.xml file, would be fine that way? Here's the commit: 8eabca9 . Thanks!

Copy link
Copy Markdown

@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.

Thanks for the changes! It all looks great :-). I'll leave the final round of stress-testing for the final PR 👍

@ThomazFB ThomazFB merged commit 62060cb into issue/11456-add-tenor-support Apr 1, 2020
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.

2 participants