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

Fix part of #494: Audio Content highlighting #514

Merged
merged 3 commits into from
Dec 5, 2019

Conversation

rt4914
Copy link
Contributor

@rt4914 rt4914 commented Dec 4, 2019

Explanation

This PR code is hard to check completely because half of the code was in #491 and other half of code is in this PR. This has happened because @BenHenning had to merge #491 in temp-integration-pt3 but that code was not 100% correct. So to test this PR, we should mainly focus on content highlighting in each and every possible case.

Also, there can be cases in which same content is getting displayed again and again, in that case on playing audio only the last card will be highlighted to keep it clean.

Also, in web, it is highlighting all the cards if the cards are repetitive, I have filed an issue for that on oppia/oppia#8127

Screenshot_1575461301
Screenshot_1575461307

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is made from a branch that is up-to-date with "develop".
  • The PR's branch is based on "develop" and not on any other branch.
  • The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

@rt4914
Copy link
Contributor Author

rt4914 commented Dec 4, 2019

@vinitamurthi @seanlip @mschanteltc @BenHenning @veena14cs @nikitamarysolomanpvt @jamesxu0 I believe that the points that I have mentioned in the description are up for discussion which lies in web domain and UX domain. Feel free to comment your suggestions.

The main question is should we highlight all the content cards in case of repetition or only the last card.

Screenshot_1575450465

Screenshot_1575461307

@mschanteltc
Copy link

@vinitamurthi @seanlip @mschanteltc @BenHenning @veena14cs @nikitamarysolomanpvt @jamesxu0 I believe that the points that I have mentioned in the description are up for discussion which lies in web domain and UX domain. Feel free to comment your suggestions.

The main question is should we highlight all the content cards in case of repetition or only the last card.

Screenshot_1575450465

Screenshot_1575461307

Let's highlight only the last card. Just wondering, is there a feature where the user can scroll up, tap a previous card, and listen to the audio of that?

Also will the scrub's cursor start at the beginning if it plays audio of a different card? We should implement this in the use case where the user answers the question incorrectly multiple times, making the time range of the scrub excessively long.

@rt4914
Copy link
Contributor Author

rt4914 commented Dec 4, 2019

@mschanteltc
Answering your questions:

  1. No currently there is no such feature. Infact the content-cards are non-clickable right now. Also, I do believe this would be a nice functionality but it might complicate things for users who are new to this because then there would a lot of items to click on screen. I mean, I think it would be a nice functionality, just not sure if the learner will get confused or not.

  2. This I am not able to understand. What do you mean by scrub?

@seanlip
Copy link
Member

seanlip commented Dec 4, 2019

"Scrub" means to drag the audio pointer horizontally along the bar.

(And @mschanteltc -- yes, the scrub starts at the beginning when the user goes to a new card.)

@mschanteltc
Copy link

mschanteltc commented Dec 4, 2019

Thanks @rt4914.

  1. This would most likely be tested in a future user study because the functionality would also rely on audio to play while the user is interacting with the app.

  2. I am referring to the audio bar and the cursor that user's can drag to change the part of the audio that is being spoken. For example, if the second card is highlighted and audio plays starting at the first word of the card, would the cursor start at the beginning of the bar? Or is the bar an accumulation of all the audio content on the page, thus making the cursor in the middle of the bar?

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!

Conflicts:
	app/src/main/java/org/oppia/app/player/state/StateFragmentPresenter.kt
@BenHenning
Copy link
Member

FYI that I'm merging this. We should resolve what we want to do from the UX/product side before finishing this work on develop.

@BenHenning BenHenning merged commit 10263a0 into temp-integration-pt3 Dec 5, 2019
@BenHenning BenHenning deleted the audio-highlighting-part-1 branch December 5, 2019 00:07
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.

8 participants