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

Fixes Issue#5853 Tapping a "... sent you an email" notification has no effect #5865

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

ErenratZeng
Copy link
Contributor

This change improves the user experience by ensuring email notifications are properly categorized and provides useful feedback when they are clicked.

Description (required)

Fixes #5853

What changes did you make and why?

This Pull request adds logic to classify notifications as "email" type when the notification text contains "sent you an email". This change ensures that email notifications are properly categorized, which was previously missing, leading to all email notifications being marked as UNKNOWN type.

Previously, email-related notifications from the backend were missing a URL. As a result, when users clicked on these notifications, there was no link to open, and the notifications were categorized as UNKNOWN. This led to a poor user experience since there was no feedback provided when the user clicked on an email notification.

Changes:

  • NotificationClient:

    • Modified WikimediaNotification.toCommonsNotification() to check if the notification text contains "sent you an email". If it does, the notification is classified as EMAIL instead of the default UNKNOWN.
  • NotificationActivity:

    • In the NotificatinAdapter click handler, added a check for EMAI type. When an email-type notification is clicked, a "Check your mail box" prompt is shown using Snackbar, instead of attempting to open a URL (which is typically missing for such notifications).
  • NotificationType:

    • Added a new EMAIL type to categorize email-related notifications.
  • Localization:

    • Added localized translations for "Check your email inbox" in values-zh/strings.xml, values-zh-rhk/strings.xml, values-zh-rcn/strings.xml, values-zh-rtw/strings.xml, and values-ja/string.xml to improve user experience for non-English speakers.

Tests performed (required)

Tested {build variant} on {Galaxy s20 SM-G9810} with API level {33}.

Screenshots (for UI changes only)
image
image
image

This commit adds logic to classify notifications as "email" type when the notification text contains "sent you an email". It also updates the email notification prompt to support localization, ensuring a better user experience across different regions.

### Problem:
1. Previously, email-related notifications from the backend were missing a URL. As a result, when users clicked on these notifications, there was no link to open, and the notifications were categorized as UNKNOWN. This led to a poor user experience since there was no feedback provided when the user clicked on an email notification.
2. Additionally, the existing code used hardcoded English text for the email notification prompt, which did not provide a localized experience for users in different regions.

### Solution:
1. Added logic to categorize email-related notifications as `EMAIL` when the notification text contains "sent you an email".
2. Replaced the hardcoded "Check your email inbox" string with a localized string and added translations for multiple languages, including zh, zh-rhk, zh-rcn, zh-rtw, and ja.

### Changes:
- **NotificationClient**:
  - Modified `WikimediaNotification.toCommonsNotification()` to check if the notification text contains "sent you an email". If it does, the notification is classified as `EMAIL_MESSAGE` instead of the default `UNKNOWN`.

- **NotificationActivity**:
  - In the `NotificatinAdapter` click handler, added a check for `EMAIL_MESSAGE` type. When an email-type notification is clicked, a localized "Check your mail box" prompt is shown using `Snackbar`, instead of attempting to open a URL (which is typically missing for such notifications).
  - Modified to fetch the string using `getString(R.string.check_your_mail_box)` to support localization.

- **NotificationType**:
  - Added a new `EMAIL` type to categorize email-related notifications.

- **Localization**:
  - Added localized translations for "Check your mail box" in zh, zh-rhk, zh-rcn, zh-rtw, and ja.
Copy link
Member

@nicolas-raoul nicolas-raoul left a comment

Choose a reason for hiding this comment

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

Working great, thanks a lot!

@nicolas-raoul nicolas-raoul merged commit c77e5ab into commons-app:main Oct 18, 2024
1 check 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.

Tapping a "... sent you an email" notification has no effect
2 participants