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

fix(notifications): Prevent notifications text from being truncated #2856

Merged
merged 4 commits into from
Jul 5, 2024

Conversation

jamilsaadeh97
Copy link
Contributor

@jamilsaadeh97 jamilsaadeh97 commented Jun 20, 2024

  • PR title and description conform to Pull Request guidelines.

Issue #, if available:
Fixes #2857
Fixes part of #4173 in the amplify flutter repo.

Description of changes:
Push notifications content are being truncated when they exceed ~90 characters and can't be shown to the user even if they expand the notification.
I've attached a screenshot on how the notification is currently shown after being expanded when using:
"This is a long push notifications message that's supposed to be completely shown on the user device" as notification body.

Screenshot 2024-06-20 at 3 30 17 PM

Per the official android documentation, I added Notification.BigTextStyle to the builder.

How did you test these changes?
I did not test this because my app is using amplify flutter, so I don't know how to reference this change since amplify-android is a transitive dependency in amplify flutter. Any suggestion would be much appreciated.

Documentation update required?

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

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.

Sorry, something went wrong.

@jamilsaadeh97 jamilsaadeh97 requested a review from a team as a code owner June 20, 2024 12:35
@vincetran
Copy link
Member

@jamilsaadeh97 Thanks for your PR! I was able to test this change locally and confirmed with stakeholders that we can pull this in. See the attached screenshots of the new experience:

after-collapsed-2
after-expanded-2

@jamilsaadeh97
Copy link
Contributor Author

@vincetran Amazing! Will be waiting for the PR to be fully approved to merge it. Thank you!

@jamilsaadeh97
Copy link
Contributor Author

Hi @vincetran Do you have any updates for moving forward regarding this PR?

@mattcreaser
Copy link
Member

After some quick internal discussions we will move ahead with this PR and open an additional feature request to expose a delegate API for configuring the notification.

We'll update this branch and then will need to have a passing test run, then we can merge!

@jamilsaadeh97
Copy link
Contributor Author

@mattcreaser It seems that AmplifyAndroid-IntegrationTest is failing but I can't know the reason. If the issue is within the scope of the PR, I'm more than happy to try to fix it.

@mattcreaser
Copy link
Member

Hi @jamilsaadeh97 I took a look and the failure wasn't caused by your changes - there are some known transient failures that can occur. I'll rerun the tests.

@mattcreaser mattcreaser enabled auto-merge (squash) July 3, 2024 19:55
@mattcreaser mattcreaser merged commit c0e5110 into aws-amplify:main Jul 5, 2024
2 of 3 checks passed
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.

Push notification body gets truncated
3 participants