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

Pride: Add new default icon #997

Merged
merged 4 commits into from
May 26, 2023
Merged

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented May 26, 2023

Description

This adds Pride icon as the default icon for the Pride month.

Warning
Targets release/7.39

Testing Instructions

  1. Install release/7.39 "release" build on a device
  2. Change the default icon
  3. Checkout this branch
  4. Run the app ("release" build ) on the device
  5. ✅ The custom icon changed in step 2 should be displayed
  6. Go to Settings > Appearance
  7. ✅ The custom icon changed in step 2 should be selected
  8. ✅ The Pride icon should be displayed after "Red Velvet" with the "Pride 2023" label
  9. Tap on it
  10. ✅ The icon should change
  11. Fresh install the app from this branch
  12. ✅ The Pride icon should be displayed
  13. Go to Settings > Appearance
  14. ✅ The Pride icon should be the selected icon

Automotive app

  1. Test that Automotive app still has the old default icon.

Instructions for 7.41

We'll revert back to the original icon in 7.41. In order to do so all that needs to be done is:

git revert 78f641f0d972af2b6215682639f498260570e00f

We'll keep the icon there in Settings though! 🏳️‍🌈

Screenshots or Screencast

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • Any jetpack compose components I added or changed are covered by compose previews

I have tested any UI changes...

  • with different themes
  • with a landscape orientation
  • with the device set to have a large display and font size
  • for accessibility with TalkBack

@ashiagr ashiagr requested a review from a team as a code owner May 26, 2023 04:30
@CookieyedCodes
Copy link

Any chance of a pride cat pockects icon? 😅

@emilylaguna emilylaguna added this to the 7.39 ❄️ milestone May 26, 2023
Copy link
Contributor

@emilylaguna emilylaguna left a comment

Choose a reason for hiding this comment

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

Nice work, I tested:

  • ✅ The default icon was changed when I upgraded from the previous version and I didn't have a custom icon chosen.
  • ✅ When I upgraded and DID have a custom icon chosen my selection was maintained
  • ✅ After reverting back to the old icon I am still able to choose Pride 2023
  • ✅ I can switch between all the other app icons

I wasn't able to test automotive but I think once that's ✅ we'll be good to go.

@mchowning
Copy link
Contributor

I wasn't able to test automotive but I think once that's ✅ we'll be good to go.

Confirmed that the icon is unchanged on Automotive, so I think this is good to go!

@ashiagr ashiagr merged commit 6283e1c into release/7.39 May 26, 2023
8 checks passed
@ashiagr ashiagr deleted the task/add-pride-icon-release-7.39 branch May 26, 2023 13:57
@gyund
Copy link
Contributor

gyund commented Jun 4, 2023

I wasn't able to test automotive but I think once that's ✅ we'll be good to go.

Confirmed that the icon is unchanged on Automotive, so I think this is good to go!

@mchowning FYI, in automotive the icon is changed. The default icon is changed to the new pride icon and the user's default/choice is ignored for automotive. Here's a screenshot from maps on mobile android 13 on a pixel 7 with the default icon selected as the default. The icon should be the usual red per user selection but it's the pride icon. Other user selections for the icon have no effect.

Screenshot_20230604-121120.png

There's chatter in Reddit about confusion with the icon change and having user choice ignored. I'm guessing it's tied to the default icon being changed.

@mchowning
Copy link
Contributor

mchowning commented Jun 4, 2023

Thanks for the heads up @gyund !

Just to make sure we're on the same page, what you're talking about is Android Auto, which is different from Android Automotive (who came up with these names, right?!?!). I believe the red icon is still showing on Android Automotive, but you're right that the pride icon is showing for Auto now.

@Pentaphon
Copy link

Pentaphon commented Jun 5, 2023

@ashiagr @mchowning there is a bug in the implementation on android:

  • you can change your launcher icon in Android but the pride icon will always be there. This differs from how it normally works. Basically makes pride the main icon and default secondary.
  • in Android automotive, it defaults to the pride icon and ignores your preferences. It is slightly confusing if you're looking for red typically and takes a few seconds to adjust.
  • you can also select the pride icon manually, giving you a single icon back

@CookieyedCodes
Copy link

@Pentaphon this has been the case since icons were introduced tho so might be a limitation of android from my understanding, but if it can be fixed then great (I'll try get an example of another open source app that dose icons in app😉) some types of people also find the icon controversial unfortunately tho

-to the devs sorry if you lose some some subs over this but please don't bow to them, do try and fix it tho :)-

@gyund
Copy link
Contributor

gyund commented Jun 5, 2023

  • in Android automotive, it defaults to the pride icon and ignores your preferences. It is slightly confusing if you're looking for red typically and takes a few seconds to adjust.

As @mchowning mentioned above, this default applies to auto not automotive. Silly naming 😂

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.

None yet

6 participants