Fixes #3896: Improve flow of lessons tab #4441
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rt4914
reviewed
Jul 22, 2022
app/src/main/java/org/oppia/android/app/topic/lessons/StorySummaryViewModel.kt
Show resolved
Hide resolved
|
For failing test cases try running tests associated with these files on Espresso + Robolectric and share the results. |
rt4914
suggested changes
Jul 30, 2022
There was a problem hiding this comment.
@vrajdesai78 The output for blind users is still not good. I have found following issues:
- When the card is selected and the spoken feedback starts the talkback says
Double Tap to Activateonly forFirst Storyand for everything else it does not sayDouble Tap to Activate. Although the double-tap does work at other places but that is not sufficient because it should get communicated correctly to user. - For
drop-down-iconit saysShow Chapter Listnormally it would be expected to have clicks for a text like that but again here it does not sayDouble Tap to Activate
Suggested Solution
- For sighted-users, the card-click should expand/collapse the list.
- For non-sighted users, the expand/collapse click functionality should be shifted to
drop-down-iconso that the Talkback will explicitly sayShow Chapter List, ... , ..., Double Tap to Activate
What do you think?
|
Updated video and code based on @rt4914 suggestion |
rt4914
suggested changes
Aug 2, 2022
rt4914
suggested changes
Aug 8, 2022
app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt
Show resolved
Hide resolved
| @@ -180,28 +184,42 @@ class TopicLessonsFragmentPresenter @Inject constructor( | |||
| ) | |||
| binding.chapterRecyclerView.adapter = createChapterRecyclerViewAdapter() | |||
|
|
|||
| binding.chapterListDropDownIcon.setOnClickListener { | |||
There was a problem hiding this comment.
Because chapterListDropDownIcon is now a clickable item it needs to be minimum 48x48 dp.
Maybe instead of using chapterListDropDownIcon we can use expandListIcon
rt4914
suggested changes
Aug 9, 2022
There was a problem hiding this comment.
@vrajdesai78 PTAL. The earlier suggest changes were two individual suggestions . If you want to apply both the changes you will need to update the logic correctly.
Also check test cases too.
app/src/main/java/org/oppia/android/app/topic/lessons/TopicLessonsFragmentPresenter.kt
Show resolved
Hide resolved
6 tasks
BenHenning
added a commit
that referenced
this pull request
Apr 11, 2023
<!-- READ ME FIRST: Please fill in the explanation section below and check off every point from the Essential Checklist! --> ## Explanation <!-- - Explain what your PR does. If this PR fixes an existing bug, please include - "Fixes #bugnum:" in the explanation so that GitHub can auto-close the issue - when this PR is merged. --> Fix #3844: When merged, this PR will; - Remove the `R.plurals.chapter_count_with_story_name` from the strings file. - Remove all translations of the `R.plurals.chapter_count_with_story_name`. - Delete the `computeStoryNameChapterCountContainerContentDescription()` method from `StorySummaryViewModel` which is no longer used. The approach to delete the above files was necessitated by a change made on [PR #4441](#4441). The PR switched from a chapter-based progress system i.e. _Chapter 1 of 10_ to a percentage-based progress system. This made the `chapter_count_with_story_name` strings and the `computeStoryNameChapterCountContainerContentDescription()` method used to generate the chapter-based progress text redundant. These have been removed to eliminate this redundancy. ## 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)). --------- Co-authored-by: Ben Henning <ben@oppia.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Explanation
Fixes #3896: Improved flow of lessons tab by removing repetitive content description.
Essential Checklist
Before
improve.mp4
After
flow.mp4