-
Notifications
You must be signed in to change notification settings - Fork 14
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(FEC-11707): V3 - cast on multiple players - when casting started by some player, "Cast" buttons become enabled on all players #642
Conversation
src/components/cast/_cast.scss
Outdated
.cast-button:hover { | ||
--disconnected-color: #{$white}; | ||
--connected-color: #{$brand-color}; | ||
.cast-button.cast-button-active { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can move this to the .player
block? and just use .cast-button-active
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can use &.cast-button-active inside the cast-button block, is that what you mean ?
wanted to separate because it seemed more readable but not critical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
src/components/cast/_cast.scss
Outdated
--connected-color: #{$brand-color}; | ||
.cast-button.cast-button-active { | ||
--connected-color: rgba(1, 172, 205, 0.8); | ||
--disconnected-color: #{$grayscale4}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is needed? same for hover
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is what needed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--disconnected-color: #{$grayscale4};
already set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, you're right
@@ -73,6 +73,7 @@ class Cast extends Component { | |||
*/ | |||
render(props: any): ?React$Element<any> { | |||
if (props.isCasting || props.isCastAvailable) { | |||
const className = props.isCasting ? `${style.castButton} ${style.castButtonActive}` : style.castButton; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const className = props.isCasting ? `${style.castButton} ${style.castButtonActive}` : style.castButton; | |
const className = props.isCasting ? [style.castButton, style.castButtonActive].join(' ') : style.castButton; |
we used to write it like this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think template string is more readable than using join()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better to be aligned but not critical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I care more about readability :)
Description of the Changes
Change cast button styles so that cast button would be highlighted only in actively casting player.
Fixes FEC-11707.
CheckLists