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 #17,#382: Hi fi topic audio bar #467

Merged
merged 18 commits into from
Nov 26, 2019

Conversation

nikitamarysolomanpvt
Copy link
Contributor

@nikitamarysolomanpvt nikitamarysolomanpvt commented Nov 21, 2019

Explanation

Hi fi topic audio bar

High-fi Checklist

  • Seekbar and its animation

Screenshot 2019-11-08 at 3 24 11 PM

  • AudioBar along with Volume icon in toolbar

Screenshot 2019-11-08 at 3 24 37 PM

Issue Link

#17

Mock Link

https://xd.adobe.com/spec/e2239cf4-9cde-4c08-5296-25316c1f0a14-9412/screen/9ae3d04d-9845-4b75-abf8-8f69e8823fb1/PM-IN-AU-Playing-2-/

GIF

new

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.

@veena14cs
Copy link
Contributor

In Pixel 3a Xl device gap is too much between play button and seekbar and also between language. Please check in the screenshot.

Screenshot_20191121-175159

@veena14cs
Copy link
Contributor

@mschanteltc can we have lighter background for audio bar or seekbar and thumb color lighter then the background. Because seekbar is not clearly visible on audiobar.

@nikitamarysolomanpvt
Copy link
Contributor Author

In Pixel 3a Xl device gap is too much between play button and seekbar and also between language. Please check in the screenshot.

Screenshot_20191121-175159

Done..

@veena14cs veena14cs assigned mschanteltc and unassigned veena14cs Nov 21, 2019
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.

This looks good and discussed with @nikitamarysolomanpvt earlier, the audiobar does have issues, which will be picked up once we get clarifications from Chantel.

@mschanteltc
Copy link

I updated the colors of the seekbar so that the played time range is white and remaining time range is #2D4A9D with a "multiply" blend mode, resulting in a #081661 color. The audio thumb is also changed to #FFFFFF with a drop shadow (#000000, opacity: 20%) with x-offset: 0, y-offset: 3, blur: 3.
Audio Scrub

@nikitamarysolomanpvt
Copy link
Contributor Author

I updated the colors of the seekbar so that the played time range is white and remaining time range is #2D4A9D with a "multiply" blend mode, resulting in a #081661 color. The audio thumb is also changed to #FFFFFF with a drop shadow (#000000, opacity: 20%) with x-offset: 0, y-offset: 3, blur: 3.

Please see the drop-shadow
opacity:54%
small
drop-shadow opacity:20%
small-2
Please let me know which one you prefer ... If you want me to use the image that you have provided .. i may use that also if this is not looking good...

@mschanteltc
Copy link

I updated the colors of the seekbar so that the played time range is white and remaining time range is #2D4A9D with a "multiply" blend mode, resulting in a #081661 color. The audio thumb is also changed to #FFFFFF with a drop shadow (#000000, opacity: 20%) with x-offset: 0, y-offset: 3, blur: 3.

Please see the drop-shadow
opacity:54%
small
drop-shadow opacity:20%
small-2
Please let me know which one you prefer ... If you want me to use the image that you have provided .. i may use that also if this is not looking good...

I think the audio thumb with the 54% opacity is looking great! It's more evident that it appears above the scrub and has a "tappable" appearance.

@nikitamarysolomanpvt
Copy link
Contributor Author

I updated the colors of the seekbar so that the played time range is white and remaining time range is #2D4A9D with a "multiply" blend mode, resulting in a #081661 color. The audio thumb is also changed to #FFFFFF with a drop shadow (#000000, opacity: 20%) with x-offset: 0, y-offset: 3, blur: 3.

Please see the drop-shadow
opacity:54%
small
drop-shadow opacity:20%
small-2
Please let me know which one you prefer ... If you want me to use the image that you have provided .. i may use that also if this is not looking good...

I think the audio thumb with the 54% opacity is looking great! It's more evident that it appears above the scrub and has a "tappable" appearance.

Ok then iam not using the svg image that i requested you.

@mschanteltc
Copy link

I updated the colors of the seekbar so that the played time range is white and remaining time range is #2D4A9D with a "multiply" blend mode, resulting in a #081661 color. The audio thumb is also changed to #FFFFFF with a drop shadow (#000000, opacity: 20%) with x-offset: 0, y-offset: 3, blur: 3.

Please see the drop-shadow
opacity:54%
small
drop-shadow opacity:20%
small-2
Please let me know which one you prefer ... If you want me to use the image that you have provided .. i may use that also if this is not looking good...

I think the audio thumb with the 54% opacity is looking great! It's more evident that it appears above the scrub and has a "tappable" appearance.

Ok then iam not using the svg image that i requested you.

LGTM

@rt4914
Copy link
Contributor

rt4914 commented Nov 25, 2019

@nikitamarysolomanpvt I tried to play the audio, the audio-bar is visible but it is not playable anymore. It maybe low-fi issue as well, but still needs some investigation.

<shape>
<solid
android:color="@color/white"/>

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this extra space.

Copy link
Contributor

@veena14cs veena14cs left a comment

Choose a reason for hiding this comment

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

LGTM one nit change.

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

@nikitamarysolomanpvt
Copy link
Contributor Author

nikitamarysolomanpvt commented Nov 26, 2019

@nikitamarysolomanpvt I tried to play the audio, the audio-bar is visible but it is not playable anymore. It maybe low-fi issue as well, but still needs some investigation.

@rt4914 Which topic you tried. Please be specific when you mention such issues... is this issue there for all topics or in specific topic?

@rt4914
Copy link
Contributor

rt4914 commented Nov 26, 2019

@nikitamarysolomanpvt I tried to play the audio, the audio-bar is visible but it is not playable anymore. It maybe low-fi issue as well, but still needs some investigation.

@rt4914 Which topic you tried. Please be specific when you mention such issues... is this issue there for all topics or in specific topic?

I checked on all topics and it was not working. But on giving a second look actually it was working on Real device and it was failing on Emulator. That's why I approved it in second pass.

@nikitamarysolomanpvt
Copy link
Contributor Author

@nikitamarysolomanpvt I tried to play the audio, the audio-bar is visible but it is not playable anymore. It maybe low-fi issue as well, but still needs some investigation.

@rt4914 Which topic you tried. Please be specific when you mention such issues... is this issue there for all topics or in specific topic?

I checked on all topics and it was not working. But on giving a second look actually it was working on Real device and it was failing on Emulator. That's why I approved it in second pass.

@rt4914 I checked and it's working perfectly on both devices and emulator... PTAL

@rt4914
Copy link
Contributor

rt4914 commented Nov 26, 2019

@nikitamarysolomanpvt I tried to play the audio, the audio-bar is visible but it is not playable anymore. It maybe low-fi issue as well, but still needs some investigation.

@rt4914 Which topic you tried. Please be specific when you mention such issues... is this issue there for all topics or in specific topic?

I checked on all topics and it was not working. But on giving a second look actually it was working on Real device and it was failing on Emulator. That's why I approved it in second pass.

@rt4914 I checked and it's working perfectly on both devices and emulator... PTAL

On further investigation it was found that it is failing on Android API 19.

@rt4914
Copy link
Contributor

rt4914 commented Nov 26, 2019

@nikitamarysolomanpvt I tried to play the audio, the audio-bar is visible but it is not playable anymore. It maybe low-fi issue as well, but still needs some investigation.

@rt4914 Which topic you tried. Please be specific when you mention such issues... is this issue there for all topics or in specific topic?

I checked on all topics and it was not working. But on giving a second look actually it was working on Real device and it was failing on Emulator. That's why I approved it in second pass.

@rt4914 I checked and it's working perfectly on both devices and emulator... PTAL

On further investigation it was found that it is failing on Android API 19.

I have filed this as an issue #475

@nikitamarysolomanpvt nikitamarysolomanpvt merged commit b5bc32b into develop Nov 26, 2019
@nikitamarysolomanpvt nikitamarysolomanpvt deleted the hi-fi-topic-audio-bar branch November 26, 2019 06:03
@mschanteltc
Copy link

I updated the colors of the seekbar so that the played time range is white and remaining time range is #2D4A9D with a "multiply" blend mode, resulting in a #081661 color. The audio thumb is also changed to #FFFFFF with a drop shadow (#000000, opacity: 20%) with x-offset: 0, y-offset: 3, blur: 3.

Please see the drop-shadow
opacity:54%
small
drop-shadow opacity:20%
small-2
Please let me know which one you prefer ... If you want me to use the image that you have provided .. i may use that also if this is not looking good...

I think the audio thumb with the 54% opacity is looking great! It's more evident that it appears above the scrub and has a "tappable" appearance.

Ok then iam not using the svg image that i requested you.

LGTM

Accessibility Scanner Results: Oppia - 2019-11-25-11:25:39

screenshot_Oppia_2019-11-25-11_25_39Item label
org.oppia.app:id/sbAudioProgress
This item may not have a label readable by screen readers.

Touch target
org.oppia.app:id/tvAudioLanguage

@mschanteltc Could you please suggest ...
This item's width is 46dp. Consider making the width of this touch target 48dp or larger.

Hi Nikita,

I fixed the language selector button to be 48 px in width. LMK what you think.

@nikitamarysolomanpvt
Copy link
Contributor Author

nikitamarysolomanpvt commented Nov 26, 2019 via email

@rt4914 rt4914 mentioned this pull request Jun 24, 2020
14 tasks
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.

5 participants