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

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

Closed
KolliAnitha opened this issue Nov 24, 2022 · 22 comments · Fixed by #5213
Closed
Assignees
Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Work: Low Solution is clear and broken into good-first-issue-sized chunks. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@KolliAnitha
Copy link

Describe the bug
In Terms of Service the website should be displayed as link and should be clickable

To Reproduce
Steps to reproduce the behavior:

  1. Go to Menu - Help
  2. Click on Terms of Service
  3. In the 1st paragraph look at the website link

Expected behavior
The link https://www.oppia.org should be displayed as a link and it should be clickable.
The sentence ' Thanks for your interest in Oppia Foundation Inc., a nonprofit that operates the website located at https://www.oppia.org (the Website).'

Website is mentioned twice in the statement.

Demonstration
If applicable, add screenshots or videos to help explain your problem.
Screenshot_20221124-143700

Environment
Device/emulator being used: Infinix smart 5A
Android or SDK version (e.g. Android 5 or SDK 21): Android version 11
App version (you can get this through system app settings or via the admin controls menu in-app): 0.10-beta-5e64fae55e

@BenHenning
Copy link
Member

This seems like a reasonable thing to fix with auto links.

@SidharthMudgil
Copy link

Can you assign this task to me?

@seanlip
Copy link
Member

seanlip commented Feb 15, 2023

@SidharthMudgil Please follow the instructions here for claiming an issue. You will need to provide an explanation of how you would fix it, and (if possible) a demo of the working fix.

I would also recommend reading and following the other instructions on that page if you haven't already done so. Thanks!

@SidharthMudgil
Copy link

Hi @BenHenning, Can I work on this issue?

Solution

Surrounding the link in a tag in the line 7 of app/src/main/res/values/terms_of_service.xml solve the issue.

Old Code

operates the website located at https://www.oppia.org (the "Website"). The

New Code

operates the website located at <a href="https://www.oppia.org">https://www.oppia.org</a> (the "Website"). The

Output

solution.mp4

@BenHenning
Copy link
Member

@SidharthMudgil this wasn't mentioned earlier, but the solution needs to be done without changing the XML files for the content (since we'll eventually be generating it based on the versions stored in https://github.com/oppia/oppia). This should be solvable without changing the XML.

@SidharthMudgil
Copy link

Hi, @BenHenning I solved the issue by adding these 2 lines in app/src/main/res/layout/policies_fragment.xml

android:linksClickable="true"
android:autoLink="web"

Output

Screenrecorder-2023-02-17-12-56-13-970.mp4

@MohitGupta121
Copy link
Member

MohitGupta121 commented Feb 20, 2023

@BenHenning as we see @pratyaksh1610 is make PR #4771 which is now Stale due to inactivity.
So can we assign this issue to @SidharthMudgil as per his solution #4760 (comment) ?

@MohitGupta121
Copy link
Member

MohitGupta121 commented Feb 20, 2023

@SidharthMudgil I think make PR for this directly if it's looks good then reviewers will automatically merge it.
What do you think @BenHenning @seanlip ?

@seanlip
Copy link
Member

seanlip commented Feb 20, 2023

Yup, it's fine to make a PR any time if there aren't any existing viable PRs for a given issue (and you can assign the contributor to the issue if you agree with the proposed solution approach).

@MohitGupta121
Copy link
Member

MohitGupta121 commented Feb 20, 2023

Okay sure @seanlip Thanks for your reply.
@SidharthMudgil please go ahead and feel free to make PR for this issue that approach you mentioned. And if you have any query then feel free to comment in the issue thread or PR thread. Thanks.

@SidharthMudgil
Copy link

SidharthMudgil commented Feb 21, 2023

Hi @BenHenning @MohitGupta121, Adding the autolink make the URL clickable but overrides the setMovementMethod property. Hence, other links like <a href="https://www.google.com">here</a> no longer work, same as if we remove the autolink the HTML links will work but the URL no longer will work. The only possible solution I can think of is to wrap all the URLs we want to make clickable around the a tag in all versions stored in https://github.com/oppia/oppia

@BenHenning
Copy link
Member

@SidharthMudgil you may need to look at how Android's TextView handles the attribute. We manually set the movement method today, so I suspect we can use something like HtmlCompat linkify programmatically to get the links to have correct URLSpans.

@SidharthMudgil
Copy link

@BenHenning How about this solution? https://stackoverflow.com/a/34793313/16177121

@BenHenning
Copy link
Member

@BenHenning How about this solution? https://stackoverflow.com/a/34793313/16177121

@SidharthMudgil I think that approach makes sense and is in-line with what I was thinking (though as a nit we should use LinkifyCompat rather than Linkify).

@shakivhussain
Copy link

shakivhussain commented Mar 24, 2023

@BenHenning As per your suggestion,
I have resolved this issue without changing XML files.

Please check my PR #4917

@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. and removed issue_type_bug labels Mar 29, 2023
@BenHenning
Copy link
Member

@shakivhussain this issue is currently assigned to @Uticodes.

@adhiamboperes adhiamboperes added Work: Low Solution is clear and broken into good-first-issue-sized chunks. work label added labels May 3, 2023
@adhiamboperes adhiamboperes added the good first issue This item is good for new contributors to make their pull request. label Oct 17, 2023
@adhiamboperes
Copy link
Collaborator

Note to developers: Please see #4918 and comments for the proposed solution to this issue.

@theMr17
Copy link
Collaborator

theMr17 commented Oct 18, 2023

@Tejas-67 Please explain how you plan to solve the issue, so that the issue can be assigned to you. As per your comment.

@Tejas-67
Copy link
Contributor

Hey @theMr17, this issue can be solved using the SpannableStringBuilder object.

@adhiamboperes
Copy link
Collaborator

@Tejas-67, in #4918, we were trying to use LinkifyCompat to solve this. Could you tell me why SpannableStringBuilder is better, or put up a draft pr with demo video to demonstrate your solution.

@Tejas-67
Copy link
Contributor

@adhiamboperes SpannableStringBuilder should be preferred because we can have the flexibility to decide what happens when the link is clicked, not just opening a link. Plus, it's easier to change the appearance of the link (colors, textStyles). Also SpannableStringBuilder is faster and more efficient because it won't scan the whole string ( we can simply pass a span range i.e. start index to end index)

@Tejas-67
Copy link
Contributor

@adhiamboperes I am facing an error
"unmappable character (0x90) for encoding windows-1252

  • Corresponds to the Arabic (اَلْعَرَب�?يَّة�?‎) macro language. IETF BCP 47 language tag: ar." . Can you please guide me on how to fix this?

Tejas-67 pushed a commit to Tejas-67/oppia-android that referenced this issue Oct 24, 2023
Tejas-67 pushed a commit to Tejas-67/oppia-android that referenced this issue Oct 24, 2023
Tejas-67 pushed a commit to Tejas-67/oppia-android that referenced this issue Oct 24, 2023
Tejas-67 pushed a commit to Tejas-67/oppia-android that referenced this issue Oct 24, 2023
Tejas-67 pushed a commit to Tejas-67/oppia-android that referenced this issue Oct 29, 2023
Tejas-67 pushed a commit to Tejas-67/oppia-android that referenced this issue Dec 5, 2023
Tejas-67 pushed a commit to Tejas-67/oppia-android that referenced this issue Dec 5, 2023
adhiamboperes added a commit that referenced this issue Dec 6, 2023
… and should be clickable. (#5213)

## 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
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] 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: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
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](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-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](https://github.com/oppia/oppia-android/assets/110051718/2d5b5bf5-3aad-43da-8b41-7146620e75ab)

---------

Co-authored-by: Tejas-67 <[email protected]>
Co-authored-by: Adhiambo Peres <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment