Fix #4760 In Terms of Service the website should be displayed as link and should be clickable. #5213

Tejas-67 · 2023-10-29T05:54:47Z

Explanation

Fix #4760 In Terms of Service the website should be displayed as link and should be clickable
This Solution uses SpannableString to solve the the issue. We get the url from html spannable using regex and then set a span around it to make the link clickable.

Essential Checklist

The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
Any changes to scripts/assets files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see RTL guide)
Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included
Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

Demo Video

oppia-.webm

adhiamboperes

Thanks @Tejas-67!

Please include a video demo of your solution working as expected.

Once you have addressed the comments, please assig this back to me. See wiki section.

app/src/main/java/org/oppia/android/app/policies/PoliciesFragmentPresenter.kt

adhiamboperes

Thanks @Tejas-67, I have added a nit, but nothing blocking. Please fix and reassign.

app/src/main/java/org/oppia/android/app/policies/PoliciesFragmentPresenter.kt

oppiabot · 2023-10-31T16:08:09Z

Hi @Tejas-67, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

adhiamboperes · 2023-11-01T09:06:45Z

Thanks @Tejas-67. Could you please fix the failing tests? See https://github.com/oppia/oppia-android/wiki/Interpreting-CI-Results for finding out which tests are failing.

Tejas-67 · 2023-11-02T12:08:29Z

Hey @adhiamboperes , fixed some lint issues that was causing the tests to fail. Other tests failing because the link to navigate to privacy policy page from terms of service page is not working. A possible fix would be to add another span for the link. Should i go ahead with this fix or raise a issue after this PR is merged?

adhiamboperes · 2023-11-02T12:15:10Z

Hey @adhiamboperes , fixed some lint issues that was causing the tests to fail. Other tests failing because the link to navigate to privacy policy page from terms of service page is not working. A possible fix would be to add another span for the link. Should i go ahead with this fix or raise a issue after this PR is merged?

@Tejas-67, all existing tests must pass for the PR to be merged. Can you identify why the links are not working? What has changed?

Tejas-67 · 2023-11-02T14:11:24Z

@adhiamboperes , The links are not working because the text is enclosed in <oppia-noninteractive-policy link="privacy">Privacy Policy and html.fromHtml doesn't recognize this. One fix could be to to wrap the text with another span.

adhiamboperes

@adhiamboperes , The links are not working because the text is enclosed in Privacy Policy and html.fromHtml doesn't recognize this. One fix could be to to wrap the text with another span.

I think this was working before your fix, right? It is worth taking a step back and finding a different solution. Take a look at this class that handles custom html tags. I believe it has a fromHtml() function that is used in the HtmlParser.kt class, which in computeCustomTagHandlers(), parses the policy tag.

adhiamboperes · 2023-11-14T16:37:29Z

@Tejas-67, are you still working on this PR?

Tejas-67 · 2023-11-15T06:32:11Z

@adhiamboperes Yes, I am working on it.

oppiabot · 2023-11-22T07:27:03Z

Hi @Tejas-67, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

Tejas-67 · 2023-11-27T18:38:34Z

Hey @adhiamboperes , please check the new code.

Tejas-67 · 2023-11-30T18:28:03Z

Hey @adhiamboperes, do I need to raise a new PR for this issue?

adhiamboperes

Thanks @Tejas-67! (Sorry for the delay in review here).

Based on previous conversation, you do not need to close a PR and open a new one, when undoing unwanted changes, you can simply create new commits after you have changed code back to the way it was, and push to the branch.

(However when working on a different issue, you need to create a new branch from develop).

Our wiki has a good set of instructions that can help. https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR. Please let me know if you have further questions.

app/src/main/res/values/terms_of_service.xml

utility/src/main/java/org/oppia/android/util/parser/html/HtmlParser.kt

adhiamboperes

Sorry, meant to request changes as per my comments.

… link and should be clickable.

- Replace brittle string manipulation with robust regex matching using .toRegex().find() - Introduce new helper functions for code simplification

Tejas-67 · 2023-12-05T18:37:14Z

@adhiamboperes please check.

adhiamboperes

Thanks @Tejas-67!

oppiabot · 2023-12-06T15:42:00Z

Hi @Tejas-67, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!

Tejas-67 requested a review from a team as a code owner October 29, 2023 05:54

Tejas-67 requested a review from adhiamboperes October 29, 2023 05:54

adhiamboperes changed the title ~~Fix oppia #4760 In Terms of Service the website should be displayed as link and should be clickable.~~ Fix #4760 In Terms of Service the website should be displayed as link and should be clickable. Oct 31, 2023

adhiamboperes requested changes Oct 31, 2023

View reviewed changes

app/src/main/java/org/oppia/android/app/policies/PoliciesFragmentPresenter.kt Outdated Show resolved Hide resolved

app/src/main/java/org/oppia/android/app/policies/PoliciesFragmentPresenter.kt Outdated Show resolved Hide resolved

adhiamboperes assigned Tejas-67 Oct 31, 2023

adhiamboperes approved these changes Oct 31, 2023

View reviewed changes

app/src/main/java/org/oppia/android/app/policies/PoliciesFragmentPresenter.kt Outdated Show resolved Hide resolved

oppiabot bot added the PR: LGTM label Oct 31, 2023

adhiamboperes removed the PR: LGTM label Nov 2, 2023

adhiamboperes requested changes Nov 8, 2023

View reviewed changes

oppiabot bot added stale Corresponds to items that haven't seen a recent update and may be automatically closed. and removed stale Corresponds to items that haven't seen a recent update and may be automatically closed. labels Nov 22, 2023

adhiamboperes assigned adhiamboperes and unassigned Tejas-67 Nov 28, 2023

Tejas-67 requested a review from a team as a code owner December 4, 2023 06:32

Tejas-67 closed this Dec 4, 2023

Tejas-67 reopened this Dec 4, 2023

adhiamboperes approved these changes Dec 4, 2023

View reviewed changes

adhiamboperes requested changes Dec 4, 2023

View reviewed changes

adhiamboperes assigned Tejas-67 and unassigned adhiamboperes Dec 4, 2023

Tejas-67 added 5 commits December 5, 2023 19:00

Fix oppia#4760 In Terms of Service the website should be displayed as…

795e45e

… link and should be clickable.

Refactor link detection in policyDescription

b47d4b2

- Replace brittle string manipulation with robust regex matching using .toRegex().find() - Introduce new helper functions for code simplification

Refractor functions to set text in textviews

ca32a64

FIX: PrivacyPolicy navigation issue.

2e48df9

Remove unused imports.

33caba8

Tejas-67 force-pushed the develop-tejas branch from bcc938d to 33caba8 Compare December 5, 2023 15:51

Applied requested changes from PR review

71cc7f0

adhiamboperes approved these changes Dec 6, 2023

View reviewed changes

Merge branch 'develop' into develop-tejas

1554e4f

adhiamboperes enabled auto-merge (squash) December 6, 2023 15:39

oppiabot bot added the PR: LGTM label Dec 6, 2023

adhiamboperes merged commit 0f8eb69 into oppia:develop Dec 6, 2023
36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #4760 In Terms of Service the website should be displayed as link and should be clickable. #5213

Fix #4760 In Terms of Service the website should be displayed as link and should be clickable. #5213

Tejas-67 commented Oct 29, 2023 •

edited by adhiamboperes

Loading

adhiamboperes left a comment

adhiamboperes left a comment

oppiabot bot commented Oct 31, 2023

adhiamboperes commented Nov 1, 2023

Tejas-67 commented Nov 2, 2023

adhiamboperes commented Nov 2, 2023

Tejas-67 commented Nov 2, 2023 •

edited

Loading

adhiamboperes left a comment

adhiamboperes commented Nov 14, 2023

Tejas-67 commented Nov 15, 2023

oppiabot bot commented Nov 22, 2023

Tejas-67 commented Nov 27, 2023

Tejas-67 commented Nov 30, 2023

adhiamboperes left a comment

adhiamboperes left a comment

Tejas-67 commented Dec 5, 2023

adhiamboperes left a comment

oppiabot bot commented Dec 6, 2023

Fix #4760 In Terms of Service the website should be displayed as link and should be clickable. #5213

Fix #4760 In Terms of Service the website should be displayed as link and should be clickable. #5213

Conversation

Tejas-67 commented Oct 29, 2023 • edited by adhiamboperes Loading

Explanation

Essential Checklist

For UI-specific PRs only

Demo Video

adhiamboperes left a comment

Choose a reason for hiding this comment

adhiamboperes left a comment

Choose a reason for hiding this comment

oppiabot bot commented Oct 31, 2023

adhiamboperes commented Nov 1, 2023

Tejas-67 commented Nov 2, 2023

adhiamboperes commented Nov 2, 2023

Tejas-67 commented Nov 2, 2023 • edited Loading

adhiamboperes left a comment

Choose a reason for hiding this comment

adhiamboperes commented Nov 14, 2023

Tejas-67 commented Nov 15, 2023

oppiabot bot commented Nov 22, 2023

Tejas-67 commented Nov 27, 2023

Tejas-67 commented Nov 30, 2023

adhiamboperes left a comment

Choose a reason for hiding this comment

adhiamboperes left a comment

Choose a reason for hiding this comment

Tejas-67 commented Dec 5, 2023

adhiamboperes left a comment

Choose a reason for hiding this comment

oppiabot bot commented Dec 6, 2023

Tejas-67 commented Oct 29, 2023 •

edited by adhiamboperes

Loading

Tejas-67 commented Nov 2, 2023 •

edited

Loading