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

Update app icons #13166

Merged
merged 11 commits into from
Dec 27, 2024
Merged

Conversation

irfano
Copy link
Contributor

@irfano irfano commented Dec 18, 2024

Closes: woocommerce/woomobile-private#374

Description

This PR updates the app icon.

Steps to reproduce

  1. Install the app on an Android 13+ device.
  2. Search for "Themed icons" in your device's settings and enable it. Note that this feature may not be available on all devices.
  3. Long tap the WooCommerce app icon and move it to the home screen.

Testing information

  • Test all three build variants
  • Test the Weap app icon
  • Test Themed icons

The tests that have been performed

Explained in "Testing information"

Images/gif

imageimageScreenshot_20241219_022831

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

@irfano irfano added the category: design Layout and style elements in the UI or user interface, including color and animations. label Dec 18, 2024
@irfano irfano added this to the 21.6 milestone Dec 18, 2024
@irfano irfano changed the base branch from trunk to feature/woo-2.0-brand-updates December 18, 2024 23:32
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This new name for ic_launcher-playstore.png has been generated by Image IDE's Asset Studio.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 18, 2024

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit6b073b2
Direct Downloadwoocommerce-wear-prototype-build-pr13166-6b073b2.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Dec 19, 2024

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit6b073b2
Direct Downloadwoocommerce-prototype-build-pr13166-6b073b2.apk

@codecov-commenter
Copy link

codecov-commenter commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.59%. Comparing base (d9724a0) to head (6b073b2).
Report is 16 commits behind head on feature/woo-2.0-brand-updates.

Additional details and impacted files
@@                         Coverage Diff                         @@
##             feature/woo-2.0-brand-updates   #13166      +/-   ##
===================================================================
- Coverage                            40.59%   40.59%   -0.01%     
+ Complexity                            6373     6372       -1     
===================================================================
  Files                                 1345     1345              
  Lines                                77235    77235              
  Branches                             10602    10602              
===================================================================
- Hits                                 31353    31352       -1     
  Misses                               43135    43135              
- Partials                              2747     2748       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JorgeMucientes JorgeMucientes self-assigned this Dec 20, 2024
@irfano irfano force-pushed the feature/woo-2.0-brand-updates branch from 5e95707 to c730abc Compare December 21, 2024 09:58
Copy link
Contributor

@JorgeMucientes JorgeMucientes left a comment

Choose a reason for hiding this comment

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

Nice job @irfano 👏🏼

I left a couple comments. One is not related to the new icons, but I asked just in case, as I think it is a concerning issue (the part about color contrast in dark mode)

The other is a minor issue since it only impacts wasabi build variant, but in any case you may want to check it :)

Anyway, I'm not blocking the PR

Comment on lines 20 to 23
val woo_purple_5 = Color(0xFFE1D7FF)
val woo_purple_10 = Color(0xFFD1C1FF)
val woo_purple_20 = Color(0xFFB999FF)
val woo_purple_alpha = Color(0x666108CE)
Copy link
Contributor

Choose a reason for hiding this comment

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

After seeing what some screens look like in dark mode, I was wondering if dark mode in mobile was considered from design when defining the new color palette. I have the feeling with the new purple palette, the color contrast is very low and doesn't fulfill the accesibility requirements

Here are a couple of examples:

  • The bottom toolbar text
  • Any CTA from the dashboard cards.
  • In essence, any text CTA

Copy link
Contributor 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, Jorge. There were some discussions on this topic, and the primary colors have been updated in #13185. Could you test that PR and leave your comments there?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh nice! Glad that the discussion about color contrast happened already and that colors were updated. The screenshots from the PR you linked look good 👍🏼 Thanks for sharing the context 🙂

@@ -1,22 +1,23 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
Copy link
Contributor

Choose a reason for hiding this comment

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

Wasabi variant is the only one where theme icons are not working. Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it should have been working for Wasabi as well. It’s working in my tests, as shown in the screenshot I shared. Are you sure you tested the correct version? Which device did you use for testing?

Copy link
Contributor

@JorgeMucientes JorgeMucientes Dec 27, 2024

Choose a reason for hiding this comment

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

Yeah, it's weird that it worked for you and not on my device. It's also really weird that only Wasabi variant didn't show the themed icon. The device I used was Android emulator Pixel 8 API 34.
Since this was merged already I'll retest again in subsequent PRs 👍🏼 Nevermind about that. I'll re-test again the PR just in case

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @irfano I re tested it and is very weird. The wasabi variant is still displayed with a colored icon. I tested in real device (Pixel 8 pro Android 15) as well (see screenshot), and the same happens as with the emulator (reported above).

Screenshot 2024-12-27 at 13 52 25

In any case, I don't think this is a blocking issue since this only happens in a non-production variant. So I don't think is a major thing. Feel free to merge or double-check the icon :).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could finally reproduce the issue and found the fix. The reason was removing -v26 from the directory name, and I added it back with 52786ff.
I don't know why it was working for me, could be related to switching from other branches.

@irfano irfano merged commit cd39850 into feature/woo-2.0-brand-updates Dec 27, 2024
15 checks passed
@irfano irfano deleted the feature/update-app-icons branch December 27, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: design Layout and style elements in the UI or user interface, including color and animations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants