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

Fixes #3906: Incorrect output for rich text in hints and solution #4666

Conversation

vrajdesai78
Copy link
Contributor

@vrajdesai78 vrajdesai78 commented Oct 19, 2022

Explanation

Fixes #3906: Fixed the overall accessibility of Hints and Solution as follows:

  • Parse HTML to normal string for content description
  • Improved flow of screen reader by removing repetitive content description.
  • Changed content description of expand_list_icon and also improve the flow by making expand_list_icon clickable and not clickable based on talkback reader.

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).

Video for Hints

hints.mp4

Video for Solution

solution.mp4

@vrajdesai78
Copy link
Contributor Author

@BenHenning @rt4914 CI checks are failing due to some flaky tests. Can you re-run it.

Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @vrajdesai78. Just had one follow-up comment--PTAL.

@BenHenning BenHenning assigned vrajdesai78 and unassigned BenHenning Oct 20, 2022
@vrajdesai78
Copy link
Contributor Author

Thanks @vrajdesai78. Just had one follow-up comment--PTAL.

@BenHenning replied to your comment, also can you please re-run CI checks. Thanks

Copy link
Member

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @vrajdesai78! This LGTM. Just waiting on CI.

@oppiabot
Copy link

oppiabot bot commented Oct 20, 2022

Unassigning @BenHenning since they have already approved the PR.

@BenHenning
Copy link
Member

CI is looking good, so I have no other concerns here. I think you just need @rt4914's approval at this point.

Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

Currently the hint-content and "Hint 1", show/hide all these are getting read-at-once.

Instead the hint-content, ie., Remember .... should be entirely separate from other things like Hint 1 and show/hide

@rt4914 rt4914 removed their assignment Oct 22, 2022
@vrajdesai78
Copy link
Contributor Author

Currently the hint-content and "Hint 1", show/hide all these are getting read-at-once.

Instead the hint-content, ie., Remember .... should be entirely separate from other things like Hint 1 and show/hide

Thanks @rt4914, I have updated code according to your suggestion. Also, updated video in the description.

Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@vrajdesai78 Suggested some more changes based on the current output.

app/src/main/res/layout/hints_summary.xml Show resolved Hide resolved
@rt4914 rt4914 removed their assignment Oct 25, 2022
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks.

@rt4914 rt4914 merged commit 9750ce9 into oppia:develop Oct 28, 2022
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.

[A11Y] Incorrect output for rich-text in Hints and Solution
3 participants