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

feat(notifications): Large images are displayed when expanding notifications #2694

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

hirotakaakita
Copy link

@hirotakaakita hirotakaakita commented Jan 30, 2024

  • PR title and description conform to Pull Request guidelines.

Issue #, if available:
None

Description of changes:
When displaying a notification with an image through Amplify, there is an issue where the attached image does not enlarge even when in the expanded state. To resolve this, I made changes to the settings based on the official Android documentation. Currently, even when the notification is in the expanded state, the appearance does not change, which I believe is different from the expected behavior by the user.

reference link:
https://developer.android.com/develop/ui/views/notifications/expanded#kotlin

How did you test these changes?
We have not confirmed whether it is accurately reflected in the UI. We executed Lint and cAT to confirm, and Lint was executed without any issues, but cAT originally failed.

Documentation update required?

  • No
  • Yes (Please include a PR link for the documentation update)

reference link:
https://docs.amplify.aws/android/build-a-backend/push-notifications/set-up-push-notifications/

General Checklist

  • Added Unit Tests
  • Added Integration Tests
  • Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@hirotakaakita hirotakaakita requested a review from a team as a code owner January 30, 2024 10:04
@mattcreaser
Copy link
Member

Hey @hirotakaakita, thanks for posting this PR! We will have a look at it and get back to you soon.

@tjleing tjleing changed the title feat(notification) Large images are displayed when expanding notifications feat(notifications): Large images are displayed when expanding notifications Feb 15, 2024
@tjleing
Copy link
Contributor

tjleing commented Feb 15, 2024

I was able to test this PR last night, and gathered these four screenshots. These first two are of the library in its current state:

before_closed
before_opened

and these two are of the library after these changes.

after_closed
after_opened

I used the pinpoint test message console, with the image URL placed in the "Android image" section.

@tjleing
Copy link
Contributor

tjleing commented Feb 17, 2024

Just tested the other scenarios and "Android icon" and "Android small icon" seem to not be used in the notifications, just the "Android image". In my opinion it makes a lot more sense to set this image as the notification image as is seen in this PR, but also that at least Android icon turns into the icon to the left, as was the behavior before.

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (1804551) 42.85% compared to head (7543a1e) 42.87%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2694      +/-   ##
==========================================
+ Coverage   42.85%   42.87%   +0.01%     
==========================================
  Files         905      905              
  Lines       29098    29102       +4     
  Branches     4142     4142              
==========================================
+ Hits        12471    12478       +7     
+ Misses      15269    15267       -2     
+ Partials     1358     1357       -1     

@hirotakaakita
Copy link
Author

@tjleing
Thank you for confirming.
The problem seems to be that setSmallIcon(R.drawable.ic_launcher_foreground) is specified.
We have resolved this issue by creating a new entrance for configuring LargeIcon.

@hirotakaakita
Copy link
Author

Sorry, I did not understand the specifications of the pinpoint test message console.
I have corrected it again.

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.

4 participants