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 #1383: added audio based content highlighting #2996

Closed
wants to merge 1 commit into from
Closed

Fix #1383: added audio based content highlighting #2996

wants to merge 1 commit into from

Conversation

justdvnsh
Copy link
Contributor

Explanation

Fix #1383: Added audio based content highlighting - to highlight the correct recyler view item based on the audio track playing

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.

…ycler view item based on the audio track playing.
@justdvnsh
Copy link
Contributor Author

@rt4914 I have created a PR as requested.
(https://drive.google.com/file/d/1w81SKafbCgszq0f9PJRZuARkEooYbREv/view?usp=sharing) this is a POC video to cross verify.

Also, there is one problem in the code. However, rest everything seems to work just fine. Please take a look and tell me, so that I can make more improvements in this PR and also, clean up the code a little.

@rt4914 rt4914 self-assigned this Mar 24, 2021
@rt4914
Copy link
Contributor

rt4914 commented Mar 30, 2021

Couple of issues with this implementation.

  1. The UI/border of the content disappears which should not happen:

  1. In case of repetitive contents only the last item should get highlighted and not all

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.

The approach is on the right track but there are a lot of edge cases that needs to be taken care of.

@rt4914 rt4914 assigned justdvnsh and unassigned rt4914 Mar 30, 2021
@justdvnsh
Copy link
Contributor Author

@rt4914 Okay. Would be working on it.

@anandwana001
Copy link
Contributor

@justdvnsh any update on this?

@justdvnsh
Copy link
Contributor Author

@anandwana001 Yes. I will be working on this from tomorrow. Sorry, got a little busy due to the GSoC and internships !!

@justdvnsh
Copy link
Contributor Author

Couple of issues with this implementation.

  1. The UI/border of the content disappears which should not happen:
  1. In case of repetitive contents only the last item should get highlighted and not all

@rt4914 Okay, so I am having a little issue here. Can you help me out a little. First part is okay (it can be solved using a binding adapter), but for the second part, there needs to be some way of highlighting only the last feedback item. Can you suggest me anything that can be done to resolve the 2nd part of this issue ?

@rt4914
Copy link
Contributor

rt4914 commented May 6, 2021

Couple of issues with this implementation.

  1. The UI/border of the content disappears which should not happen:
  1. In case of repetitive contents only the last item should get highlighted and not all

@rt4914 Okay, so I am having a little issue here. Can you help me out a little. First part is okay (it can be solved using a binding adapter), but for the second part, there needs to be some way of highlighting only the last feedback item. Can you suggest me anything that can be done to resolve the 2nd part of this issue ?

@justdvnsh Actually even I do not have any direct solution here because that's the same step I was stuck the last time. Do you have some approaches in mind?

@anandwana001 anandwana001 added the PR don't merge - NEEDS UPDATE Corresponds to PRs that need to be updated with the latest develop changes before they can be merged label May 7, 2021
@anandwana001
Copy link
Contributor

@justdvnsh any update on this?

@justdvnsh
Copy link
Contributor Author

@anandwana001 Yup. I am stuck too on the step mentioned above. Trying to formulate a solution for now.

@justdvnsh
Copy link
Contributor Author

justdvnsh commented May 13, 2021

@rt4914 Not

Couple of issues with this implementation.

  1. The UI/border of the content disappears which should not happen:
  1. In case of repetitive contents only the last item should get highlighted and not all

@rt4914 Okay, so I am having a little issue here. Can you help me out a little. First part is okay (it can be solved using a binding adapter), but for the second part, there needs to be some way of highlighting only the last feedback item. Can you suggest me anything that can be done to resolve the 2nd part of this issue ?

@justdvnsh Actually even I do not have any direct solution here because that's the same step I was stuck the last time. Do you have some approaches in mind?

Not really Actually ! I tried a few things. But none of them seem to work

@anandwana001 anandwana001 assigned rt4914 and unassigned rt4914 May 19, 2021
@anandwana001
Copy link
Contributor

@rt4914 PTAL

@anandwana001
Copy link
Contributor

Closing this PR as no further update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR don't merge - NEEDS UPDATE Corresponds to PRs that need to be updated with the latest develop changes before they can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Audio based Content Item highlighting in StateFragment
3 participants