Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Adjust min threshold for play button #7685

Merged
merged 1 commit into from
Mar 15, 2017
Merged

Adjust min threshold for play button #7685

merged 1 commit into from
Mar 15, 2017

Conversation

cezaraugusto
Copy link
Contributor

@cezaraugusto cezaraugusto commented Mar 13, 2017

Fix #7665

Auditors: @bbondy

Preview:

new_bpoint_audio_tab

Note: I had to remove a test case since for all medimLarge sizes we no longer show the default play indicator

Test Plan:

Tabs playing audio with less than 120px should have alternative play indicator (top blue border)

@cezaraugusto cezaraugusto added this to the 0.13.6 milestone Mar 13, 2017
@cezaraugusto cezaraugusto self-assigned this Mar 13, 2017
@cezaraugusto cezaraugusto requested a review from bbondy March 13, 2017 18:44
@cezaraugusto
Copy link
Contributor Author

We can also benefit by reducing play button size. /cc @bradleyrichter

@bbondy
Copy link
Member

bbondy commented Mar 14, 2017

Could you make this a bit more, more? Maybe 120px

@bradleyrichter
Copy link
Contributor

We may want to consider a swap with favicon --> mute button?

The percentage of mutable tabs is low, but recognizing quickly which tab is making noise is probably more important than recognizing the favicon.

@cezaraugusto
Copy link
Contributor Author

how would that work with alternative audio indicator (top blue border)?

@bradleyrichter
Copy link
Contributor

we may not need the audio indicator. (which BTW needs testing with new styling branch since the shadow you were using is now gone. : )

@cezaraugusto
Copy link
Contributor Author

audio is working fine. I'm ok with favicon replacement, but we should consider favicon-only cases for small sizes, where I think top blue border would apply. Otherwise we'll not be able to change tabs without muting. Thoughts?

@bradleyrichter
Copy link
Contributor

bradleyrichter commented Mar 14, 2017 via email

@bbondy
Copy link
Member

bbondy commented Mar 15, 2017

much better, thank you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants