Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Update screenshare button background when active",
"packageName": "@internal/react-components",
"email": "2684369+JamesBurnside@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "Moved code",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we be specific in this comment about which code was removed?
Helps when we go over the combined changelog later.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh cool. Didn't know that.

"packageName": "@internal/react-composites",
"email": "2684369+JamesBurnside@users.noreply.github.com",
"dependentChangeType": "none"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "update jest snapshots",
"packageName": "@internal/storybook",
"email": "2684369+JamesBurnside@users.noreply.github.com",
"dependentChangeType": "none"
}
33 changes: 7 additions & 26 deletions packages/react-components/src/components/ControlBarButton.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import React, { useCallback } from 'react';
import {
DefaultButton,
IButtonProps,
IRenderFunction,
Text,
concatStyleSets,
mergeStyles,
IButtonStyles
} from '@fluentui/react';
import { useTheme } from '../theming';
import { controlButtonLabelStyles, controlButtonStyles } from './styles/ControlBar.styles';
import React from 'react';
import { DefaultButton, IButtonProps, IRenderFunction, concatStyleSets, IButtonStyles } from '@fluentui/react';
import { controlButtonStyles } from './styles/ControlBar.styles';
import { ControlButtonTooltip } from './ControlButtonTooltip';

/**
Expand Down Expand Up @@ -123,22 +114,10 @@ const DefaultRenderIcon = (props?: ControlBarButtonProps): JSX.Element | null =>
*/
export const ControlBarButton = (props: ControlBarButtonProps): JSX.Element => {
const componentStyles = concatStyleSets(controlButtonStyles, props.styles ?? {});
const theme = useTheme();

const labelText =
props?.text ?? props?.strings?.label ?? (props?.checked ? props?.strings?.onLabel : props?.strings?.offLabel);

const DefaultRenderText = useCallback(() => {
return (
<Text
key={props?.labelKey}
className={mergeStyles(controlButtonLabelStyles, theme.palette.neutralPrimary, props?.styles?.label)}
>
{labelText}
</Text>
);
}, [labelText, props?.labelKey, props?.styles?.label, theme]);

const tooltipContent =
props?.strings?.tooltipContent ??
(props?.disabled
Expand All @@ -154,9 +133,11 @@ export const ControlBarButton = (props: ControlBarButtonProps): JSX.Element => {
<DefaultButton
{...props}
styles={componentStyles}
onRenderText={props.showLabel ? props.onRenderText ?? DefaultRenderText : undefined}
onRenderText={props.showLabel && props.onRenderText ? props.onRenderText : undefined}
onRenderIcon={props.onRenderIcon ?? DefaultRenderIcon}
/>
>
{props.showLabel ? labelText : <></>}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

NOTE: No longer using a custom render function (so get the styling from props correctly)

</DefaultButton>
</ControlButtonTooltip>
);
};
21 changes: 20 additions & 1 deletion packages/react-components/src/components/ScreenShareButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import React from 'react';
import { useLocale } from '../localization';
import { ControlBarButton, ControlBarButtonProps } from './ControlBarButton';
import { Icon } from '@fluentui/react';
import { DefaultPalette, IButtonStyles, Icon, Theme, useTheme } from '@fluentui/react';

/**
* Strings of {@link ScreenShareButton} that can be overridden.
Expand Down Expand Up @@ -56,9 +56,13 @@ export const ScreenShareButton = (props: ScreenShareButtonProps): JSX.Element =>
const localeStrings = useLocale().strings.screenShareButton;
const strings = { ...localeStrings, ...props.strings };

const theme = useTheme();
const styles = screenshareButtonStyles(theme);

return (
<ControlBarButton
{...props}
styles={styles}
onClick={props.onToggleScreenShare ?? props.onClick}
onRenderOnIcon={props.onRenderOnIcon ?? onRenderScreenShareOnIcon}
onRenderOffIcon={props.onRenderOffIcon ?? onRenderScreenShareOffIcon}
Expand All @@ -67,3 +71,18 @@ export const ScreenShareButton = (props: ScreenShareButtonProps): JSX.Element =>
/>
);
};

const screenshareButtonStyles = (theme: Theme): IButtonStyles => ({
Copy link
Copy Markdown
Member Author

@JamesBurnside JamesBurnside Nov 29, 2021

Choose a reason for hiding this comment

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

NOTE: Now in components and not composites so our customers get this behavior

rootChecked: {
background: theme.palette.themePrimary,
color: DefaultPalette.white,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

curious about the usage of DefaultPalette for color instead of theme?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think @PorterNan worked on this originally.

':focus::after': { outlineColor: `${DefaultPalette.white} !important` } // added !important to avoid override by FluentUI button styles
},
rootCheckedHovered: {
background: theme.palette.themePrimary,
color: DefaultPalette.white,
':focus::after': { outlineColor: `${DefaultPalette.white} !important` } // added !important to avoid override by FluentUI button styles
},
labelHovered: { color: DefaultPalette.white },
labelChecked: { color: DefaultPalette.white }
});
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,17 @@ export const controlButtonStyles: IButtonStyles = {
flexContainer: {
flexFlow: 'column',
display: 'contents'
},
label: {
fontSize: '0.625rem',
fontWeight: '400',
lineHeight: '1rem',
cursor: 'pointer',
display: 'block',
margin: '0rem 0.25rem'
}
};

/**
* @private
*/
export const controlButtonLabelStyles: IStyle = {
fontSize: '0.625rem',
lineHeight: '1rem',
cursor: 'pointer',
display: 'block',
margin: '0rem 0.25rem'
};

/**
* making it Partial as IContextualMenuStyles has all its props non-optional and we only need title to be defined here.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,14 @@ import {
MicrophoneButton,
ParticipantMenuItemsCallback,
ParticipantsButton,
ScreenShareButton,
useTheme
ScreenShareButton
} from '@internal/react-components';
import React, { useMemo } from 'react';
import { useLocale } from '../../localization';
import { usePropsFor } from '../hooks/usePropsFor';
import { useSelector } from '../hooks/useSelector';
import { getCallStatus, getLocalMicrophoneEnabled } from '../selectors/baseSelectors';
import {
checkedButtonOverrideStyles,
controlButtonBaseStyle,
devicesButtonWithIncreasedTouchTargets,
groupCallLeaveButtonCompressedStyle,
Expand Down Expand Up @@ -128,13 +126,6 @@ export const CallControls = (props: CallControlsProps): JSX.Element => {
const devicesButtonProps = usePropsFor(DevicesButton);
const hangUpButtonProps = usePropsFor(EndCallButton);

const theme = useTheme();

const screenShareButtonStyles = useMemo(
() => mergeButtonBaseStyles(checkedButtonOverrideStyles(theme, screenShareButtonProps.checked)),
[screenShareButtonProps.checked, theme.palette.themePrimary]
);

const participantsButtonStyles = useMemo(
() => mergeButtonBaseStyles(props.increaseFlyoutItemSize ? participantButtonWithIncreasedTouchTargets : {}),
[props.increaseFlyoutItemSize]
Expand Down Expand Up @@ -170,7 +161,6 @@ export const CallControls = (props: CallControlsProps): JSX.Element => {
<ScreenShareButton
data-ui-id="call-composite-screenshare-button"
{...screenShareButtonProps}
styles={screenShareButtonStyles}
showLabel={!compactMode}
disabled={options?.screenShareButton !== true && options?.screenShareButton?.disabled}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { DefaultPalette as palette, IButtonStyles, IContextualMenuItemStyles, IStyle, Theme } from '@fluentui/react';
import { IButtonStyles, IContextualMenuItemStyles, IStyle } from '@fluentui/react';
import { DevicesButtonStyles, ParticipantsButtonStyles } from '@internal/react-components';

const MINIMUM_TOUCH_TARGET_HEIGHT_REM = 3;
Expand Down Expand Up @@ -46,18 +46,6 @@ export const groupCallLeaveButtonCompressedStyle = {
}
};

/**
* @private
*/
export const checkedButtonOverrideStyles = (theme: Theme, isChecked?: boolean): IButtonStyles => ({
rootChecked: {
background: theme.palette.themePrimary,
color: palette.white,
':focus::after': { outlineColor: `${palette.white} !important` } // added !important to avoid override by FluentUI button styles
},
label: isChecked ? { color: palette.white } : {}
});

/**
* Styles that can be applied to ensure flyout items have the minimum touch target size.
*
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ exports[`storybook snapshot tests Storyshots UI Components/Control Bar Control B
onMouseLeave={[Function]}
>
<button
className="ms-Button ms-Button--default root-5"
className="ms-Button ms-Button--default root-13"
data-is-focusable={true}
onClick={[Function]}
onKeyDown={[Function]}
Expand Down Expand Up @@ -248,7 +248,7 @@ exports[`storybook snapshot tests Storyshots UI Components/Control Bar Control B
</i>
<i
aria-hidden={true}
className="ms-Icon root-37 css-13 ms-Button-menuIcon menuIcon-10"
className="ms-Icon root-37 css-14 ms-Button-menuIcon menuIcon-10"
data-icon-name="ChevronDown"
hidden={true}
style={
Expand Down Expand Up @@ -313,7 +313,7 @@ exports[`storybook snapshot tests Storyshots UI Components/Control Bar Control B
</i>
<i
aria-hidden={true}
className="ms-Icon root-37 css-13 ms-Button-menuIcon menuIcon-10"
className="ms-Icon root-37 css-14 ms-Button-menuIcon menuIcon-10"
data-icon-name="ChevronDown"
hidden={true}
style={
Expand All @@ -336,7 +336,7 @@ exports[`storybook snapshot tests Storyshots UI Components/Control Bar Control B
onMouseLeave={[Function]}
>
<button
className="ms-Button ms-Button--default root-14"
className="ms-Button ms-Button--default root-15"
data-is-focusable={true}
onClick={[Function]}
onKeyDown={[Function]}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,14 @@ exports[`storybook snapshot tests Storyshots Examples/Teams Interop/Lobby Lobby
</span>
</i>
<span
className="#323130 css-23"
className="ms-Button-textContainer textContainer-16"
>
Turn off
<span
className="ms-Button-label label-18"
id="id__2"
>
Turn off
</span>
</span>
</span>
</button>
Expand Down Expand Up @@ -198,9 +203,14 @@ exports[`storybook snapshot tests Storyshots Examples/Teams Interop/Lobby Lobby
</span>
</i>
<span
className="#323130 css-23"
className="ms-Button-textContainer textContainer-16"
>
Mute
<span
className="ms-Button-label label-18"
id="id__6"
>
Mute
</span>
</span>
</span>
</button>
Expand All @@ -214,7 +224,7 @@ exports[`storybook snapshot tests Storyshots Examples/Teams Interop/Lobby Lobby
onMouseLeave={[Function]}
>
<button
className="ms-Button ms-Button--default root-24"
className="ms-Button ms-Button--default root-22"
data-is-focusable={true}
onClick={[Function]}
onKeyDown={[Function]}
Expand Down Expand Up @@ -252,13 +262,18 @@ exports[`storybook snapshot tests Storyshots Examples/Teams Interop/Lobby Lobby
</span>
</i>
<span
className="#323130 css-23"
className="ms-Button-textContainer textContainer-16"
>
Devices
<span
className="ms-Button-label label-18"
id="id__10"
>
Devices
</span>
</span>
<i
aria-hidden={true}
className="ms-Icon root-37 css-26 ms-Button-menuIcon menuIcon-25"
className="ms-Icon root-37 css-24 ms-Button-menuIcon menuIcon-23"
data-icon-name="ChevronDown"
hidden={true}
style={
Expand All @@ -281,7 +296,7 @@ exports[`storybook snapshot tests Storyshots Examples/Teams Interop/Lobby Lobby
onMouseLeave={[Function]}
>
<button
className="ms-Button ms-Button--default root-27"
className="ms-Button ms-Button--default root-25"
data-is-focusable={true}
onClick={[Function]}
onKeyDown={[Function]}
Expand Down Expand Up @@ -325,9 +340,14 @@ exports[`storybook snapshot tests Storyshots Examples/Teams Interop/Lobby Lobby
</span>
</i>
<span
className="#323130 css-30"
className="ms-Button-textContainer textContainer-16"
>
Leave
<span
className="ms-Button-label label-26"
id="id__14"
>
Leave
</span>
</span>
</span>
</button>
Expand Down
Loading