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

#4083 fixed external link warning when user wants to make a donation #4133

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

Conversation

viditpawar0
Copy link

Fixes #4083

Fixed external link warning when user wants to make a donation

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.

@viditpawar0 We have the DonationPopup functionality as well which we have recently added, which shows on the reader screen. So when the user clicks on the "Make a donation" button this method calls

override fun openExternalUrl(intent: Intent) {

According to your existing code, it will work fine with the Kiwix app, but for custom apps, we have overrided this method in CustomReaderFragment to make custom behavior according to configuration. So we have to make the changes there as well so that when while user clicks on the donation button in custom apps this popup will not show.

Apart from these changes: The test cases are falling due to this change specially topLevelDestinationTest so can you please fix that?

@viditpawar0
Copy link
Author

viditpawar0 commented Dec 11, 2024

Sure, I'll do that.
I'm not sure about the topLevelDestinationTest though.

@viditpawar0
Copy link
Author

All the requested changes have been done.

Summary: -

  • made the same fix for custom apps in CustomMainActivity
  • modified topLevelDestinationTest
  • fixed the same for CoreReaderFragment and CustomReaderFragment

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.

@viditpawar0 Thanks for making the changes. Please see comments.

@@ -123,7 +123,6 @@ class TopLevelDestinationTest : BaseActivityTest() {
clickSettingsOnSideNav(SettingsRobot::assertMenuSettingsDisplayed)
clickHelpOnSideNav(HelpRobot::assertToolbarDisplayed)
clickSupportKiwixOnSideNav()
assertExternalLinkDialogDisplayed()
Copy link
Collaborator

Choose a reason for hiding this comment

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

The same we are using in the GetContentShortcutTest so please make the changes there as well.

@@ -186,7 +186,7 @@ class CustomMainActivity : CoreMainActivity() {
private fun openExternalUrl(url: String) {
// check if the provided url is not empty.
if (url.isNotEmpty()) {
externalLinkOpener.openExternalUrl(url.toUri().browserIntent())
externalLinkOpener.openExternalUrl(url.toUri().browserIntent(), false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@viditpawar0 This code is used from two places, so your code works fine when someone wants to make a donation, but it does not show the external link popup if there is about_app URL is configured. It should show the external link popup for the about_app URL.

screen-20241212-112625.mp4

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.

Do we really want to warn the user who wants to make a donation?
3 participants