Skip to content

Change default app icon to spectrum#18638

Merged
guarani merged 1 commit intorelease/19.9from
update/app-icon-to-spectrum
May 20, 2022
Merged

Change default app icon to spectrum#18638
guarani merged 1 commit intorelease/19.9from
update/app-icon-to-spectrum

Conversation

@guarani
Copy link
Copy Markdown
Contributor

@guarani guarani commented May 17, 2022 •
edited
Loading

This PR is similar to #16673 and sets the default icon for Pride Month (internal reference).

App Before After
Jetpack
WordPress

I expect we'll be doing a follow-up PR in June (for v20.1) to update this back to the "Cool Blue" icon.

To test

Jetpack app

This PR changes the app icon from the default green icon to the new icon shown above. Note that the Jetpack app does not support custom icons.

Verify app icon

  • Install the Jetpack build
  • Verify that the icon is now the new icon

WordPress app

This PR:

  • Adds the new icon as the default (users who have already changed the app icon will not have their preference changed)
  • Names the new icon Pride '22
  • Adds the "Cool Blue" icon, which was the default, to the App Icon screen in case users want to switch to it

The following tests are inspired by last year's PR which updated the WordPress icon similarly: #16673

No existing custom icon scenario

  • Install a previous build of the app. Do not select a custom icon.
  • Install this build on top of it. You should see the app icon gets updated to the new icon.
  • Visit the Me > App Settings > App Icon settings and ensure that new icon is checked at the top and you can choose an alternative (including "Cool Blue").

Existing custom icon scenario

  • Follow the steps above, but before installing this build, set a custom icon to something else. Ensure that setting is respected when upgrading.

Downgrading scenario

  • Follow the steps in the no existing custom icon scenario.
  • After installing this build and running the app, install a previous build again over the top.
  • The icon should still be "Cool Blue" and if you check the the App Icon screen, it should reflect that at the top of the list.

Reverting after switching back to blue

  • Follow the steps in the no existing custom icon scenario, and then manually switch your icon back to blue in the app settings.
  • Install a previous build again over the top.
  • The icon should revert back to "Cool Blue" and the App Icon screen should reflect that choice at the top of the list.

Regression Notes

  1. Potential unintended areas of impact

Unintended areas of impact could be anything related to the app's default icon not working, or issues changing the icon.

  1. What I did to test those areas of impact (or what existing automated tests I relied on)

The above manual tests.

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

There's no practical way to add automated tests to these changes, because they don't affect app functionality

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.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented May 17, 2022 •
edited
Loading

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18638-4232dec on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented May 17, 2022 •
edited
Loading

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr18638-4232dec on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@guarani guarani changed the title Change defaultapp icon to spectrum Change default app icon to spectrum May 17, 2022
@guarani guarani force-pushed the update/app-icon-to-spectrum branch 4 times, most recently from 2d5907c to 9a4d7f9 Compare May 18, 2022 22:43
@guarani
Copy link
Copy Markdown
Contributor Author

guarani commented May 19, 2022 •
edited
Loading

Testing build 9a4d7f9

Jetpack app

  • Verify app icon: I see new icon ✅

WordPress app

  • No existing custom icon scenario: ❌ After installing the new build on top of the old one, the app icon isn't updated on the device home screen until I restart my device. Before restarting, I notice the icon was still "Cool Blue", although I'd see the new icon for split-second during the animation as I back-grounded the app.
    See screen recording
  • Existing custom icon scenario
  • Downgrading scenario: ❌ The issue from the "No existing custom icon scenario" is present here as well, both when upgrading and downgrading.
    See screen recording
  • Reverting after switching back to blue

frosty
frosty approved these changes May 19, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@frosty frosty left a comment

Choose a reason for hiding this comment

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

Jetpack app

✅ Updated as expected, required a restart of my simulator

WordPress app

No existing custom icon scenario

✅ Updated as expected, required a restart of my simulator

Existing custom icon scenario

✅ Same icon kept, as expected

Downgrading scenario

✅ Reverted to Cool Blue as expected, required a restart

Reverting after switching back to blue

✅ Reverted to Cool Blue as expected.

Looks good to me!

guarani
guarani commented May 19, 2022
View reviewed changes
@guarani guarani changed the base branch from trunk to release/19.9 May 19, 2022 19:50
Change default WP app icon to Spectrum 2022
4232dec 
This is a temporary change to make Spectrum the default WP app icon. This is a seasonal change, so we expect to revert this change in an upcoming app version.
@guarani
Copy link
Copy Markdown
Contributor Author

guarani commented May 19, 2022

Thanks for the review, @frosty! I'd left it as a draft and hadn't finished the testing myself, but I appreciate you giving this a review and helping out with this.

The base branch was still trunk and I needed to change it to release/19.4, which involves rebasing this branch and that might invalidate comments. Normally I wouldn't force-push after a review, but because this is still a draft I'll have to. I hope that makes sense and sorry for the confusion.

On the plus side, the rebase looks to have removed the unwanted changes to the project.pbxproj.

@guarani guarani force-pushed the update/app-icon-to-spectrum branch from 9a4d7f9 to 4232dec Compare May 19, 2022 20:03
guarani
guarani commented May 19, 2022
View reviewed changes
@guarani
Copy link
Copy Markdown
Contributor Author

guarani commented May 19, 2022 •
edited
Loading

Testing build 4232dec on iPad Pro 12.9" 5th gen simulator (iOS 15.2).

Jetpack app

  • Verify app icon

WordPress app

  • No existing custom icon scenario †
  • Existing custom icon scenario
  • Downgrading scenario †
  • Reverting after switching back to blue †

† Note: While the issue from #18638 (comment) persists (where the icon gets "stuck" on the old one until the device is restarted), we discussed this in p1652973042939329/1652884477.028099-slack-C011BKNU1V5 and think this issue might be an iOS issue rather than an issue with the app. I'll be testing the Test Flight build that includes this PR when it comes out, to see if this issue is resolved there. Please let me know if you have any concerns @mokagio

@guarani guarani requested a review from frosty May 19, 2022 21:49
@guarani guarani marked this pull request as ready for review May 19, 2022 21:50
@guarani
Copy link
Copy Markdown
Contributor Author

guarani commented May 19, 2022 •
edited
Loading

Hi @frosty, I re-requested a review after rebasing, although I don't believe it's necessary for you to run through the test cases again, but it's up to you. I would also appreciate your thoughts on #18638 (comment) 🙇.

To give you a sense of how I've tested this, my previous tests were on a physical iPhone using the branch build. Today, after the rebase, I tested this on an iPad simulator. As noted above, I plan to check the 19.9 TestFlight build that includes this PR when it comes out, to see if the "stuck icon" issue is fixed there. I'll be doing that on a physical iPhone device.

Thanks for all the help so far, @frosty!

@guarani guarani added this to the 19.9 ❄️ milestone May 19, 2022
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented May 19, 2022

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

@guarani
Copy link
Copy Markdown
Contributor Author

guarani commented May 19, 2022

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

I don't think this change will have release notes since 19.9 is frozen cc @mokagio

@mokagio
Copy link
Copy Markdown
Contributor

mokagio commented May 20, 2022

I don't think this change will have release notes since 19.9 is frozen cc @mokagio

@guarani , correct

@mattmiklic
Copy link
Copy Markdown
Member

mattmiklic commented May 20, 2022

Everything looks good and worked as expected. 🚀

@guarani
Copy link
Copy Markdown
Contributor Author

guarani commented May 20, 2022

Thank you @mattmiklic!

@guarani guarani mentioned this pull request May 20, 2022
Closed
@guarani
Copy link
Copy Markdown
Contributor Author

guarani commented May 20, 2022

Hi @frosty, I re-requested a review after rebasing, although I don't believe it's necessary for you to run through the test cases again, but it's up to you. I would also appreciate your thoughts on #18638 (comment) 🙇.

On second thought, I think a re-review might not be necessary since all that's changed between your last review and now is that I've rebased this branch from trunk to the release branch. I've also replied to #18638 (comment)

I'd like to go ahead and merge this if you don't mind @frosty.

@guarani guarani removed the request for review from frosty May 20, 2022 20:40
@guarani guarani merged commit 47d9e74 into release/19.9 May 20, 2022
@guarani guarani deleted the update/app-icon-to-spectrum branch May 20, 2022 20:41
@guarani
Copy link
Copy Markdown
Contributor Author

guarani commented May 20, 2022

I've merged this given your earlier approval @frosty, and my testing as well as the testing from @mattmiklic.
cc @mokagio

@mokagio mokagio mentioned this pull request May 23, 2022
Merged
7 tasks
@mokagio
Copy link
Copy Markdown
Contributor

mokagio commented May 23, 2022

@guarani this has been bundled as part of 19.9 beta 3 (19.9.0.3).

Thanks for your work 🙌

Also, the new icons came through correctly in TestFlight 👍

IMG_5726D0B197B6-1

IMG_06320256285F-1

@guarani
Copy link
Copy Markdown
Contributor Author

guarani commented May 23, 2022

@guarani this has been bundled as part of 19.9 beta 3 (19.9.0.3).

Thank you @mokagio! ❤️

Also, the new icons came through correctly in TestFlight 👍

That's great to know, I appreciate you checking! 🙇

@frosty frosty mentioned this pull request Jun 24, 2022
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@frosty frosty frosty approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

19.9

Development

Successfully merging this pull request may close these issues.

5 participants

@guarani @wpmobilebot @mokagio @mattmiklic @frosty