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

Fix checkmark fading animation when choosing a site design #17868

Merged
merged 2 commits into from Feb 4, 2022

Conversation

twstokes
Copy link
Contributor

@twstokes twstokes commented Feb 2, 2022

Fixes #16690

Description

This PR corrects a minor issue where the white background view behind the blue checkmark image would cause a flickering effect due to how their opacities would blend. The solution proposed here is to combine both views into one container view.

A 3D view of the view hierarchy

To test

Choosing a design

  1. In the "My Sites" view, tap the "+" symbol at the top right.
  2. Choose "Create WordPress.com site".
  3. Tap different designs to toggle the checkmark icon animation.
  4. Observe that the checkmark's location is the same relative location as before to the top right corner of its view.

Choosing a layout

Prerequisites: This view may be shown with all configurations, but definitely works with WordPress.com sites and a theme from WordPress (e.g. Alves)

  1. In the "My Sites" view, tap Pages, then the blue button to create a new page.
  2. Tap the blue button to show the "Choose a Layout" view.
  3. Tap different layouts to toggle the checkmark icon animation.
  4. Observe that the checkmark's location is the same relative location as before to the top right corner of its view.

Before

Simulator.Screen.Recording.-.iPhone.13.-.2022-02-02.at.17.02.46.mp4

After

Simulator.Screen.Recording.-.iPhone.13.-.2022-02-02.at.16.59.57.mp4

Other notes

Eventually we can move to UIImage.SymbolConfiguration (needs iOS 15+) and define the symbol's colors, which is a simpler solution.

Regression Notes

  1. Potential unintended areas of impact
  • The site design picker when creating a new WordPress.com site from the app.
  1. What I did to test those areas of impact (or what existing automated tests I relied on)
    I tested the steps above manually on a device.

  2. What automated tests I added (or what prevented me from doing so)
    None.

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.

@twstokes twstokes added this to the 19.2 milestone Feb 2, 2022
@twstokes twstokes self-assigned this Feb 2, 2022
@jkmassel
Copy link
Contributor

jkmassel commented Feb 2, 2022

You can test the Jetpack changes on this Pull Request by downloading it from AppCenter here with build number: pr17868-a001683. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@jkmassel
Copy link
Contributor

jkmassel commented Feb 2, 2022

You can test the WordPress changes on this Pull Request by downloading it from AppCenter here with build number: pr17868-a001683. IPA is available here. If you need access to this, you can ask a maintainer to add you.

@peril-wordpress-mobile
Copy link

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

@antonis antonis self-requested a review February 4, 2022 08:12
Copy link
Member

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Great work @twstokes 👍
I tested this on a iPhone SE 2020 (iOS 14.7.1) and everything worked as expected. The code changes also look consistent to me 🎉

I also validated that the fix also applies to the page layout picker selector 👍

Page layout picker before
before.mov
Page layout picker after
after.mov

@peril-wordpress-mobile
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@twstokes
Copy link
Contributor Author

twstokes commented Feb 4, 2022

I also validated that the fix also applies to the page layout picker selector 👍

Thanks for highlighting that @antonis! I'll update the testing steps to include that view as well. 🙇

@twstokes twstokes merged commit 443de8e into trunk Feb 4, 2022
@twstokes twstokes deleted the bug/checkmark-fading branch February 4, 2022 15:43
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.

very tiny visual bug: Blue selected state flickers when choosing a design
3 participants