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

#4236 Improved download notification strings replacing default Fetch #4241

Closed

Conversation

amanna13
Copy link

@amanna13 amanna13 commented Feb 27, 2025

Fixes #4236

📌 Summary

  1. Improving notification wording by replacing "starting" with "resuming" when a paused download is resuming
  2. Replacing Default Fetch library strings with our own string resources to handle translation in other languages.

@MohitMaliFtechiz
Copy link
Collaborator

@amanna13 The PR is ready for review?

@amanna13
Copy link
Author

@amanna13 The PR is ready for review?

Yes !

@MohitMaliFtechiz MohitMaliFtechiz self-requested a review February 28, 2025 06:54
Copy link
Collaborator

@MohitMaliFtechiz MohitMaliFtechiz left a comment

Choose a reason for hiding this comment

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

Thanks for your changes, please see the review changes.

@@ -402,4 +402,9 @@
<string name="donation_dialog_title">Donate Today</string>
<string name="donation_dialog_description">%s needs your help.</string>
<string name="make_donation">Make a donation</string>
<string name="fetch_notification_download_complete">Complete</string>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@amanna13 We should give our name to these strings instead of fetch, and it could lead to unexpected behavior when accessing it at runtime. See in the below image we are using this string, and the R class we have imported in this class(DownloadMonitorService) from the fetch so this string will not take ours. So to avoid confusion we should rename it. Additionally, there are already some strings available e.g. Complete, Paused we should use the existing ones.

image

else -> super.getSubtitleText(context, downloadNotification)
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

After renaming the string name, please use our string in the getCancelNotification method of the FetchNotificationManager class so that it will use our text instead of fetch.

@amanna13 amanna13 closed this Feb 28, 2025
@amanna13 amanna13 deleted the 4236-improve-download-notification-strings branch February 28, 2025 15:11
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.

Improve notification wording
2 participants