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

Stories: prevent gif files from being added as background for new Story slides #13287

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Nov 4, 2020

Fixes Automattic/stories-android#592: the issue describes vido, but it's actually a gif file what the root cause of the problem was.

When a gif file is attempted to be used as a background for a Story slide, this PR shows this dialog:

Screen Shot 2020-11-04 at 08 37 36

(notet: open to suggestions about wording there)

To test:
First of all, make sure to add a gif file to your site's media library using tenor. Go to the media section of the app, press the + button, then select tenor as a source.

CASE A: create a story with 1 slide (gif)

  1. create a new story by tapping on the WPAndroid app's main screen FAB -> Story
  2. when the picker appears, select the W icon on the right bottom of the screen
  3. select the gif file only and tap the ✔️ on the navigation bar
  4. observe the story composer appears and then the dialog shows up indicating it's not possible to add gif files as a slide background for now

CASE B: create a story using 1 gif and 1 (or more) normal image / video

  1. create a new story by tapping on the WPAndroid app's main screen FAB -> Story
  2. when the picker appears, select the W icon on the right bottom of the screen
  3. select the gif file and a few other supported files (images / videos) and tap the ✔️ on the navigation bar
  4. observe the story composer appears and then the dialog shows up indicating it's not possible to add gif files as a slide background for now, but the Story with the valid slides is kept for you to continue creating
    nogif

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.

@mzorz mzorz added this to the 16.2 milestone Nov 4, 2020
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Nov 4, 2020

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Nov 4, 2020

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

Copy link
Contributor

@jd-alexander jd-alexander 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 these changes @mzorz Works as expected! My only suggested changes will be with the text of the dialog.

@mzorz
Copy link
Contributor Author

mzorz commented Nov 4, 2020

Thanks for these changes @mzorz Works as expected! My only suggested changes will be with the text of the dialog.

Excellent! thanks for the suggestions, I agree and just applied them 👍 - ready for another round / approval @jd-alexander 🙇

@aforcier
Copy link
Contributor

aforcier commented Nov 4, 2020

Since Joel already tested I'll just drive by and 👍 the dialog text with Joel's suggestions. 🎉

@mzorz
Copy link
Contributor Author

mzorz commented Nov 4, 2020

Awesome! Merging then, thank you both @jd-alexander @aforcier 🙇

@mzorz mzorz merged commit b5c38ef into feature/jetpack-stories-block-base Nov 4, 2020
@mzorz mzorz deleted the fix/stories-prevent-adding-gif-background branch November 4, 2020 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants