Fix control bar button label styling. Fix screenshare button hover and checked styling.#1178
Conversation
JamesBurnside
requested review from
PorterNan,
alcail,
alkwa-msft,
anjulgarg,
mgamis-msft and
prprabhu-ms
as code owners
| ); | ||
| }; | ||
|
|
||
| const screenshareButtonStyles = (theme: Theme): IButtonStyles => ({ |
There was a problem hiding this comment.
NOTE: Now in components and not composites so our customers get this behavior
| onRenderIcon={props.onRenderIcon ?? DefaultRenderIcon} | ||
| /> | ||
| > | ||
| {props.showLabel ? labelText : <></>} |
There was a problem hiding this comment.
NOTE: No longer using a custom render function (so get the styling from props correctly)
…om/Azure/communication-ui-library into jaburnsi/screenshare-btn-hover-fix
|
Awesome. |
| @@ -0,0 +1,7 @@ | |||
| { | |||
| "type": "none", | |||
| "comment": "Moved code", | |||
There was a problem hiding this comment.
Can we be specific in this comment about which code was removed?
Helps when we go over the combined changelog later.
There was a problem hiding this comment.
This is a no-op for this package (nothing changes from the perspective of the composites package) so type is 'none'. When the changelog is generated it omits these
| const screenshareButtonStyles = (theme: Theme): IButtonStyles => ({ | ||
| rootChecked: { | ||
| background: theme.palette.themePrimary, | ||
| color: DefaultPalette.white, |
There was a problem hiding this comment.
curious about the usage of DefaultPalette for color instead of theme?
There was a problem hiding this comment.
I left this as is (this code was just moved from one file to another) but also curious on this too if anyone knows why default palette is used
Confirmed both, thanks! |
3 participants
What
Why
https://skype.visualstudio.com/SPOOL/_workitems/edit/2633102
How Tested
Locally tested:
Will check UI tests once they complete.
Tested keyboard nav