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

Improve notification wording #4236

Open
kelson42 opened this issue Feb 26, 2025 · 5 comments
Open

Improve notification wording #4236

kelson42 opened this issue Feb 26, 2025 · 5 comments
Assignees

Comments

@kelson42
Copy link
Collaborator

Screenshot_20250226-113921.png

Replace "starting" with "resuming"

@kelson42 kelson42 changed the title Screenshot (26 Feb 2025 11:39:21) Improve notification wording Feb 26, 2025
@amanna13
Copy link

amanna13 commented Feb 26, 2025

@kelson42 I'm working on this to change the notification text from "Starting" to "Resuming". The text comes from getSubtitleText() in DefaultFetchNotificationManager, which is part of the Fetch library.
Since this function is from Fetch, should we override it in Kiwix with a custom FetchNotificationManager, or is there another preferred way to handle this?

@kelson42
Copy link
Collaborator Author

@MohitMaliFtechiz is the right person to answer this question. We also have to propose translations of these strings.

@amanna13
Copy link

Thanks, @kelson42!
@MohitMaliFtechiz, I have modified the logic to properly handle isQueued without overriding DefaultFetchNotificationManager. Instead of relying on Fetch’s default behavior, I explicitly check if downloadNotification.isQueued and then display "Resuming".

The existing default Fetch string remains unchanged. Do I need to provide translations for the string in all locales, or should I proceed with creating a pull request for now?

val subtitleText = when { downloadNotification.isQueued -> context.getString(R.string.fetch_notification_download_starting) else -> getSubtitleText(context, downloadNotification) }

@MohitMaliFtechiz
Copy link
Collaborator

MohitMaliFtechiz commented Feb 27, 2025

@amanna13 Thank you for your debugging in this issue. Here we should override the getSubtitleText method in our FetchDownloadNotificationManager class like we are overriding registerBroadcastReceiver and updateNotification methods.

@MohitMaliFtechiz, I have modified the logic to properly handle isQueued without overriding DefaultFetchNotificationManager. Instead of relying on Fetch’s default behavior, I explicitly check if downloadNotification.isQueued and then display "Resuming".

@amanna13 The best is to override the getSubtitleText method and return the text from our string resource file for all scenarios not just for resuming the download. By doing this, we can properly handle the translation of these strings in other languages(Which will automatically done by the TW). Currently, in the fetch library, they are only handling these strings in the English language.

Do I need to provide translations for the string in all locales, or should I proceed with creating a pull request for now?

No, you don't need to translate it in all locales, it will be automatically translated by the TW. You just need to add it in the English version. After that, please make a PR.

Edited

Apart from this, in FetchDownloadNotificationManager we are using some other strings from the Fetch library e.g. fetch_notification_download_paused in getCancelNotification, and fetch_notification_download_complete in showDownloadCompletedNotification method of DownloadMonitorService class. We should use our string instead of Fetch's string for proper translations. @amanna13 Can you please do it?

@amanna13
Copy link

Thanks for the guide, @MohitMaliFtechiz . Yeah sure I can update them to use our own string resources for better translation handling.

amanna13 added a commit to amanna13/kiwix-android that referenced this issue Feb 27, 2025
amanna13 added a commit to amanna13/kiwix-android that referenced this issue Feb 27, 2025
amanna13 added a commit to amanna13/kiwix-android that referenced this issue Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants