Icon rounding issue resolved #6770

rafaysaleem89 · 2021-12-15T11:31:17Z

Details

The android push notification icon will now be rounded. For some reason the push notifications weren't working on the debug build, so had to do my tests on release builds.

Fixed Issues

The Custom Notification Provider was being used to make a custom layout for notifications in which a bitmap was being used for user's avatar which was being displayed in a. square shape. I added the code to clip the bitmap over a circular path to fix this issue.
$ #6132

Tests

Log in with account A on Android
Log in with account B on web
Send some message from account B to account A
Check the push notifications now the user avatar is rounded.

QA Steps

Add a numbered list of manual tests that can be performed by our QA engineers on the staging environment to validate that 1. Log in with account A on Android
2. Log in with account B on web
3. Send some message from account B to account A
4. Check the push notifications now the user avatar is rounded.

Tested On

Android

Screenshots

Add screenshots for all platforms tested. Pull requests won't be merged unless the screenshots show the app was
tested on all platforms

(This was an Android only issue hence only added screen shot of Android)

Android

github-actions · 2021-12-15T11:31:35Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

parasharrajat · 2021-12-15T11:36:54Z

@rafaysaleem89 Could you please sign the CLA?

parasharrajat · 2021-12-15T11:38:03Z

@Julesssss Could you please review this? i

rafaysaleem89 · 2021-12-15T12:21:26Z

I have read the CLA Document and I hereby sign the CLA

Julesssss · 2021-12-15T15:25:49Z

android/app/src/main/java/com/expensify/chat/CustomNotificationProvider.java

+            bitmap.getHeight(), Config.ARGB_8888);
+       Canvas canvas = new Canvas(output);
+
+       final int color = 0xff424242;


What colour? Improve var name please.

Julesssss · 2021-12-15T15:27:35Z

android/app/src/main/java/com/expensify/chat/CustomNotificationProvider.java


+    public Bitmap getCroppedBitmap(Bitmap bitmap) {


Please add a comment to the function explaining what it's doing.

rafaysaleem89 · 2021-12-16T14:04:35Z

Updated

Julesssss · 2021-12-17T16:51:55Z

Hi @rafaysaleem89, the code looks good but I struggled with building to native Android devices (some dependency issues, ect). I'll aim to build and confirm Monday morning.

marcaaron · 2021-12-17T22:08:12Z

Sorry this code looks good but I'm not able to get push notifications working on Android at all and can't seem to figure out why. Also won't be able to get to this until next week.

marcaaron · 2021-12-17T22:08:35Z

Guessing they are just not working on dev and need to use the staging server.

marcaaron · 2021-12-17T22:21:15Z

Oh LOL I recently removed my SIM card so I think they can't work anymore. Going to unassign myself here as I'm unable to test.

Julesssss · 2021-12-20T21:03:37Z

I had further trouble today and didn't get around to this, but I did resolve the release build issues. Back on this tomorrow!

parasharrajat

Not an Android guy but the code looks neat. and have one question. I leave reviewing and merging to @Julesssss. thanks.

parasharrajat · 2021-12-21T14:51:19Z

android/app/src/main/java/com/expensify/chat/CustomNotificationProvider.java

+            bitmap.getHeight(), Config.ARGB_8888);
+       Canvas canvas = new Canvas(output);
+
+       final int defaultBackgroundColor = 0xff424242;


Is this color used only for masking? If this is shown on the notification then we need to make it theme-specific.
e.g. Dark mode

This color is the fill color for the circle that will be drawn in the background. I don't think that it will ever be shown as there will always be a bitmap drawn on top of it. But we could make it theme specific just in case.

Julesssss

I was expecting to see the user's actual profile picture, but this isn't the case. Is that expected @rafaysaleem89?

Android 8
No profile image is displayed

Android 12
Profile image is displayed, but it is not the actual custom image

Julesssss · 2021-12-21T17:18:02Z

The sender has a custom profile image, which I was expecting to see here. Perhaps this was never implemented and should be treated as a separate issue though.

rafaysaleem89 · 2021-12-21T19:25:07Z

The sender has a custom profile image, which I was expecting to see here. Perhaps this was never implemented and should be treated as a separate issue though.

this issue might be linked to this part of the code. there is a notification cache in which user information is being saved, and hence it may be that the profile picture is updated later on and it is being retrieved from the cache instead of actually getting it from the server.

rafaysaleem89 · 2021-12-27T09:32:47Z

@Julesssss how do you want to proceed on this?

Julesssss

Code looks fine to me.

Julesssss · 2022-01-04T12:19:33Z

Hi @rafaysaleem89, sorry for the delay, I have been off over Christmas and NY.

I should have merged previously, as clearly the missing profile is an unrelated issue.

OSBotify · 2022-01-04T12:20:09Z

@rafaysaleem89, Great job getting your first Expensify/App pull request over the finish line! 🎉

I know there's a lot of information in our contributing guidelines, so here are some points to take note of 📝:

Now that your first PR has been merged, you can be hired for another issue. Once you've completed a few issues, you may be eligible to work on more than one job at a time.
Once your PR is deployed to our staging servers, it will undergo quality assurance (QA) testing. If we find that it doesn't work as expected or causes a regression, you'll be responsible for fixing it. Typically, we would revert this PR and give you another chance to create a similar PR without causing a regression.
Once your PR is deployed to production, we start a 7-day timer ⏰. After it has been on production for 7 days without causing any regressions, then we pay out the Upwork job. 💰

So it might take a while before you're paid for your work, but we typically post multiple new jobs every day, so there's plenty of opportunity. I hope you've had a positive experience contributing to this repo! 😊

OSBotify · 2022-01-04T13:03:00Z

🚀 Deployed to staging by @Julesssss in version: 1.1.24-21 🚀

platform	result
🤖 android 🤖	failure ❌
🖥 desktop 🖥	failure ❌
🍎 iOS 🍎	failure ❌
🕸 web 🕸	failure ❌

Julesssss · 2022-01-04T14:27:41Z

This failure is due to an existing issue that has now been fixed. At the time of the merge, main hadn't recieved the hotfix.

mvtglobally · 2022-01-04T19:45:13Z

@Julesssss @parasharrajat
On what platforms do you need this tested? Android app only or any other

Julesssss · 2022-01-05T11:37:23Z

Hi @mvtglobally, just Android please.

OSBotify · 2022-01-05T23:58:21Z

🚀 Deployed to production by @francoisl in version: 1.1.25-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

parasharrajat · 2023-01-11T10:16:26Z

Heads up! This PR caused an issue related to Avatar images on android #12967.

Note for me: Although, I am assigned to C+ here Neither I tested/approved it nor did I get paid for this. I handed it over to Jules #6770 (comment) due to unfamiliarity with the native code.

Icon rounding issue resolved

6be374d

rafaysaleem89 requested a review from a team as a code owner December 15, 2021 11:31

MelvinBot requested review from marcaaron and parasharrajat and removed request for a team December 15, 2021 11:31

rafaysaleem89 mentioned this pull request Dec 15, 2021

[HOLD for payment 2022-01-12] Android - Add a round profile image on push notifications #6132

Closed

Julesssss assigned rafaysaleem89 Dec 15, 2021

Julesssss reviewed Dec 15, 2021

View reviewed changes

variable name improved and comments added

e2d73b6

marcaaron removed their request for review December 17, 2021 22:21

parasharrajat reviewed Dec 21, 2021

View reviewed changes

Julesssss reviewed Dec 21, 2021

View reviewed changes

Julesssss approved these changes Jan 4, 2022

View reviewed changes

Julesssss merged commit 63a7f55 into Expensify:main Jan 4, 2022

OSBotify mentioned this pull request Jan 4, 2022

Deploy Checklist: New Expensify 2022-01-04 #7005

Closed

45 tasks

parasharrajat mentioned this pull request Jan 11, 2023

[HOLD for payment 2023-01-05] [HOLD #13392] [$1000] Android - Avatar is not displayed on push notification #12967

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Icon rounding issue resolved #6770

Icon rounding issue resolved #6770

rafaysaleem89 commented Dec 15, 2021 •

edited by Julesssss

Loading

github-actions bot commented Dec 15, 2021 •

edited

Loading

parasharrajat commented Dec 15, 2021

parasharrajat commented Dec 15, 2021

rafaysaleem89 commented Dec 15, 2021

Julesssss Dec 15, 2021

Julesssss Dec 15, 2021

rafaysaleem89 commented Dec 16, 2021

Julesssss commented Dec 17, 2021 •

edited

Loading

marcaaron commented Dec 17, 2021

marcaaron commented Dec 17, 2021

marcaaron commented Dec 17, 2021

Julesssss commented Dec 20, 2021

parasharrajat left a comment

parasharrajat Dec 21, 2021

rafaysaleem89 Dec 21, 2021

Julesssss left a comment •

edited

Loading

Julesssss commented Dec 21, 2021

rafaysaleem89 commented Dec 21, 2021

rafaysaleem89 commented Dec 27, 2021

Julesssss left a comment

Julesssss commented Jan 4, 2022

OSBotify commented Jan 4, 2022

OSBotify commented Jan 4, 2022

Julesssss commented Jan 4, 2022 •

edited

Loading

mvtglobally commented Jan 4, 2022

Julesssss commented Jan 5, 2022

OSBotify commented Jan 5, 2022

parasharrajat commented Jan 11, 2023

Icon rounding issue resolved #6770

Icon rounding issue resolved #6770

Conversation

rafaysaleem89 commented Dec 15, 2021 • edited by Julesssss Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Android

github-actions bot commented Dec 15, 2021 • edited Loading

parasharrajat commented Dec 15, 2021

parasharrajat commented Dec 15, 2021

rafaysaleem89 commented Dec 15, 2021

Julesssss Dec 15, 2021

Choose a reason for hiding this comment

Julesssss Dec 15, 2021

Choose a reason for hiding this comment

rafaysaleem89 commented Dec 16, 2021

Julesssss commented Dec 17, 2021 • edited Loading

marcaaron commented Dec 17, 2021

marcaaron commented Dec 17, 2021

marcaaron commented Dec 17, 2021

Julesssss commented Dec 20, 2021

parasharrajat left a comment

Choose a reason for hiding this comment

parasharrajat Dec 21, 2021

Choose a reason for hiding this comment

rafaysaleem89 Dec 21, 2021

Choose a reason for hiding this comment

Julesssss left a comment • edited Loading

Choose a reason for hiding this comment

Julesssss commented Dec 21, 2021

rafaysaleem89 commented Dec 21, 2021

rafaysaleem89 commented Dec 27, 2021

Julesssss left a comment

Choose a reason for hiding this comment

Julesssss commented Jan 4, 2022

OSBotify commented Jan 4, 2022

OSBotify commented Jan 4, 2022

Julesssss commented Jan 4, 2022 • edited Loading

mvtglobally commented Jan 4, 2022

Julesssss commented Jan 5, 2022

OSBotify commented Jan 5, 2022

parasharrajat commented Jan 11, 2023

rafaysaleem89 commented Dec 15, 2021 •

edited by Julesssss

Loading

github-actions bot commented Dec 15, 2021 •

edited

Loading

Julesssss commented Dec 17, 2021 •

edited

Loading

Julesssss left a comment •

edited

Loading

Julesssss commented Jan 4, 2022 •

edited

Loading