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

ToggleButton: Re-introducing ToggleButton using the latest version of makeStyles #17566

Merged
merged 8 commits into from
Mar 26, 2021
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
59 changes: 53 additions & 6 deletions apps/vr-tests/src/stories/ReactButton.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { storiesOf } from '@storybook/react';
import * as React from 'react';
import Screener from 'screener-storybook/src/screener';
import { Button, ButtonProps, CompoundButton } from '@fluentui/react-button';
import { Button, ButtonProps, CompoundButton, ToggleButton } from '@fluentui/react-button';

import { FluentProviderDecorator, FabricDecorator } from '../utilities/index';

Expand Down Expand Up @@ -106,8 +106,6 @@ storiesOf('react-button CompoundButton', module)
.mouseDown('button')
.snapshot('pressed', { cropTo: '.testWrapper' })
.mouseUp('button')
.executeScript("document.getElementsByTagName('button')[0].focus()")
.snapshot('focus', { cropTo: '.testWrapper' })
.end()}
>
{story()}
Expand Down Expand Up @@ -151,8 +149,57 @@ storiesOf('react-button CompoundButton', module)
Hello, world
</CompoundButton>
))
.addStory('Icon only', () => (
<CompoundButton secondaryContent="This is some secondary text" icon="X">
.addStory('Icon only', () => <CompoundButton icon="X" />);

storiesOf('react-button ToggleButton', module)
.addDecorator(FabricDecorator)
.addDecorator(FluentProviderDecorator)
.addDecorator(story => (
<Screener
steps={new Screener.Steps()
.snapshot('default', { cropTo: '.testWrapper' })
.hover('button')
.snapshot('hover', { cropTo: '.testWrapper' })
.mouseDown('button')
.snapshot('pressed', { cropTo: '.testWrapper' })
.mouseUp('button')
.end()}
>
{story()}
</Screener>
))
.addStory('Default', () => <ToggleButton>Hello, world</ToggleButton>)
.addStory('Primary', () => <ToggleButton primary>Hello, world</ToggleButton>)
.addStory('Disabled', () => <ToggleButton disabled>Hello, world</ToggleButton>)
.addStory('Primary Disabled', () => (
<ToggleButton primary disabled>
Hello, world
</CompoundButton>
</ToggleButton>
))
.addStory('With icon before content', () => <ToggleButton icon="X">Hello, world</ToggleButton>)
.addStory('With icon after content', () => (
<ToggleButton icon="X" iconPosition="after">
Hello, world
</ToggleButton>
))
.addStory('Size small', () => (
<ToggleButton icon="X" size="large">
Hello, world
</ToggleButton>
))
.addStory('Size large', () => (
<ToggleButton icon="X" size="large">
Hello, world
</ToggleButton>
))
.addStory('Icon only', () => <ToggleButton icon="X" />)
.addStory('Checked', () => (
<ToggleButton icon="X" checked>
Hello, world
</ToggleButton>
))
.addStory('Primary Checked', () => (
<ToggleButton icon="X" primary checked>
Hello, world
</ToggleButton>
));
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "ToggleButton: Re-introducing ToggleButton using the latest version of makeStyles.",
"packageName": "@fluentui/react-button",
"email": "Humberto.Morimoto@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "ToggleButton: Re-introducing ToggleButton using the latest version of makeStyles.",
"packageName": "@fluentui/react-examples",
"email": "Humberto.Morimoto@microsoft.com",
"dependentChangeType": "patch"
}
67 changes: 63 additions & 4 deletions packages/react-button/etc/react-button.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ export type ButtonTokens = {
background: string;
borderColor: string;
color: string;
shadow: string;
}>;
pressed: Partial<{
background: string;
Expand All @@ -84,13 +85,29 @@ export type ButtonTokens = {
};

// @public (undocumented)
export type ButtonVariants = 'base' | 'disabled' | 'iconOnly' | 'primary' | 'small' | 'large' | 'primaryDisabled' | 'iconOnlySmall' | 'iconOnlyLarge';
export type ButtonVariants = 'base' | 'disabled' | 'iconOnly' | 'primary' | 'small' | 'large' | 'disabledPrimary' | 'iconOnlySmall' | 'iconOnlyLarge';

// @public (undocumented)
export type ButtonVariantTokens = {
[variant in ButtonVariants]: Partial<ButtonTokens>;
};

// @public (undocumented)
export interface CheckedState {
// (undocumented)
'aria-checked'?: React.AriaAttributes['aria-pressed'];
// (undocumented)
'aria-pressed'?: React.AriaAttributes['aria-pressed'];
// (undocumented)
checked?: boolean;
// (undocumented)
defaultChecked?: boolean;
// (undocumented)
onClick?: React.DOMAttributes<HTMLElement>['onClick'];
// (undocumented)
role?: string;
}

// @public
export const CompoundButton: React.ForwardRefExoticComponent<CompoundButtonProps & React.RefAttributes<HTMLElement>>;

Expand Down Expand Up @@ -126,16 +143,49 @@ export type CompoundButtonTokens = ButtonTokens & CompoundButtonBaseTokens & {
export type CompoundButtonVariants = ButtonVariants;

// @public (undocumented)
export type CompoundButtonVariantTokens = {
export type CompoundButtonVariantTokens = Partial<{
[variant in CompoundButtonVariants]: Partial<CompoundButtonTokens>;
};
}>;

// @public
export const renderButton: (state: ButtonState) => JSX.Element;
const renderButton: (state: ButtonState) => JSX.Element;

export { renderButton }

export { renderButton as renderToggleButton }

// @public
export const renderCompoundButton: (state: CompoundButtonState) => JSX.Element;

// @public
export const ToggleButton: React.ForwardRefExoticComponent<ToggleButtonProps & React.RefAttributes<HTMLElement>>;

// @public (undocumented)
export interface ToggleButtonProps extends ButtonProps {
checked?: boolean;
defaultChecked?: boolean;
}

// @public (undocumented)
export interface ToggleButtonState extends Omit<ToggleButtonProps, 'children' | 'icon'>, ButtonState {
}

// @public (undocumented)
export type ToggleButtonStyleSelectors = ButtonStyleSelectors & {
checked?: boolean;
};

// @public (undocumented)
export type ToggleButtonTokens = ButtonTokens;

// @public (undocumented)
export type ToggleButtonVariants = ButtonVariants | 'checked' | 'checkedPrimary';

// @public (undocumented)
export type ToggleButtonVariantTokens = Partial<{
[variant in ToggleButtonVariants]: Partial<ToggleButtonTokens>;
}>;

// @public
export const useButton: (props: ButtonProps, ref: React.Ref<HTMLElement>, defaultProps?: ButtonProps | undefined) => ButtonState;

Expand All @@ -145,12 +195,21 @@ export const useButtonState: (draftState: ButtonState) => void;
// @public (undocumented)
export const useButtonStyles: (state: ButtonState, selectors: ButtonStyleSelectors) => void;

// @public
export const useChecked: <TState extends CheckedState>(state: TState) => void;

// @public
export const useCompoundButton: (props: CompoundButtonProps, ref: React.Ref<HTMLElement>, defaultProps?: CompoundButtonProps | undefined) => CompoundButtonState;

// @public (undocumented)
export const useCompoundButtonStyles: (state: CompoundButtonState, selectors: import("../Button").ButtonStyleSelectors) => void;

// @public (undocumented)
export const useToggleButton: (props: ToggleButtonProps, ref: React.Ref<HTMLElement>, defaultProps?: ToggleButtonProps | undefined) => ToggleButtonState;

// @public (undocumented)
export const useToggleButtonStyles: (state: ToggleButtonState, selectors: ToggleButtonStyleSelectors) => void;


// (No @packageDocumentation comment for this package)

Expand Down
1 change: 1 addition & 0 deletions packages/react-button/src/ToggleButton.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './components/ToggleButton/index';
2 changes: 1 addition & 1 deletion packages/react-button/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ export const Button = React.forwardRef<HTMLElement, ButtonProps>((props, ref) =>

const styleSelectors: ButtonStyleSelectors = {
disabled: state.disabled,
primary: state.primary,
iconOnly: receivedIcon && !receivedChildren,
primary: state.primary,
size: state.size,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export type ButtonTokens = {
background: string;
borderColor: string;
color: string;
shadow: string;
}>;

pressed: Partial<{
Expand All @@ -173,7 +174,7 @@ export type ButtonVariants =
| 'small'
| 'large'
// TODO: get rid of these combinations, use individual variants in matchers
| 'primaryDisabled'
| 'disabledPrimary'
| 'iconOnlySmall'
| 'iconOnlyLarge';

Expand Down
27 changes: 18 additions & 9 deletions packages/react-button/src/components/Button/useButtonStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ export const makeButtonTokens = (theme: Theme): ButtonVariantTokens => ({
borderRadius: theme.global.borderRadius.medium,
borderWidth: theme.global.strokeWidth.thin,

// TODO: spec calls out "shadow 4 __lighter__", are we missing tokens?
shadow: theme.alias.shadow.shadow4,

fontSize: theme.global.type.fontSizes.base[300],
fontWeight: theme.global.type.fontWeights.semibold,
lineHeight: theme.global.type.lineHeights.base[300],
Expand All @@ -45,12 +48,16 @@ export const makeButtonTokens = (theme: Theme): ButtonVariantTokens => ({
background: theme.alias.color.neutral.neutralBackground1Hover,
borderColor: theme.alias.color.neutral.neutralStroke1Hover,
color: theme.alias.color.neutral.neutralForeground1,
// TODO: spec calls out "shadow 4 __lighter__", are we missing tokens?
shadow: theme.alias.shadow.shadow4,
},

pressed: {
background: theme.alias.color.neutral.neutralBackground1Pressed,
borderColor: theme.alias.color.neutral.neutralStroke1Pressed,
color: theme.alias.color.neutral.neutralForeground1,
// TODO: spec calls out "shadow 2 __lighter__", are we missing tokens?
shadow: theme.alias.shadow.shadow2,
},
},
disabled: {
Expand Down Expand Up @@ -122,17 +129,19 @@ export const makeButtonTokens = (theme: Theme): ButtonVariantTokens => ({
background: theme.alias.color.brand.brandBackgroundHover,
borderColor: 'transparent',
color: theme.alias.color.neutral.neutralForegroundInvertedAccessible,
Copy link
Member

Choose a reason for hiding this comment

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

This in particular seems weird - background is using brand and foreground is using something which doesn't relate to brand. Shouldn't foregrounds and backgrounds use the same color group?

Copy link
Member Author

Choose a reason for hiding this comment

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

@dzearing and I spoke offline about this and it does seem like we are missing some logic in here to add the brand foreground colors. There should also probably be a way to catch that we're not using foreground and background colors from the same alias group on any one variant that we also need to follow up with.

// TODO: spec calls out "shadow 4 __brand__", are we missing tokens?
shadow: theme.alias.shadow.shadow4,
},

pressed: {
background: theme.alias.color.brand.brandBackgroundPressed,
borderColor: 'transparent',
color: theme.alias.color.neutral.neutralForegroundInvertedAccessible,
// TODO: spec calls out "shadow 2 __darker__", are we missing tokens?
// TODO: spec calls out "shadow 2 __brand__", are we missing tokens?
shadow: theme.alias.shadow.shadow2,
},
},
primaryDisabled: {
disabledPrimary: {
background: theme.alias.color.neutral.neutralBackgroundDisabled,
// borderColor: theme.alias.color.neutral.neutralStrokeDisabled,
color: theme.alias.color.neutral.neutralForegroundDisabled,
Expand Down Expand Up @@ -251,19 +260,19 @@ const useStyles = makeStyles({
// TODO: focus
};
},
rootPrimaryDisabled: theme => {
rootDisabledPrimary: theme => {
const buttonTokens = makeButtonTokens(theme);

return {
background: buttonTokens.primaryDisabled.background,
color: buttonTokens.primaryDisabled.color,
boxShadow: buttonTokens.primaryDisabled.shadow,
background: buttonTokens.disabledPrimary.background,
color: buttonTokens.disabledPrimary.color,
boxShadow: buttonTokens.disabledPrimary.shadow,
':hover': {
background: buttonTokens.primaryDisabled.background,
background: buttonTokens.disabledPrimary.background,
cursor: 'default',
},
':active': {
boxShadow: buttonTokens.primaryDisabled.pressed?.shadow,
boxShadow: buttonTokens.disabledPrimary.pressed?.shadow,
},
};
},
Expand Down Expand Up @@ -359,7 +368,7 @@ export const useButtonStyles = (state: ButtonState, selectors: ButtonStyleSelect
selectors.primary && styles.rootPrimary,
selectors.size === 'small' && styles.rootSmall,
selectors.size === 'large' && styles.rootLarge,
selectors.primary && selectors.disabled && styles.rootPrimaryDisabled,
selectors.disabled && selectors.primary && styles.rootDisabledPrimary,
selectors.iconOnly && selectors.size === 'small' && styles.rootIconOnlySmall,
selectors.iconOnly && selectors.size === 'large' && styles.rootIconOnlyLarge,
state.className,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as React from 'react';
import { CompoundButtonProps, CompoundButtonStyleSelectors } from './CompoundButton.types';
import { renderCompoundButton } from './renderCompoundButton';
import { useCompoundButton } from './useCompoundButton';
import { useCompoundButtonStyles } from './useCompoundButtonStyles';
import { renderCompoundButton } from './renderCompoundButton';

/**
* Define a styled CompoundButton, using the `useCompoundButton` hook.
Expand All @@ -16,8 +16,8 @@ export const CompoundButton = React.forwardRef<HTMLElement, CompoundButtonProps>

const styleSelectors: CompoundButtonStyleSelectors = {
disabled: state.disabled,
primary: state.primary,
iconOnly: receivedIcon && !receivedChildren,
primary: state.primary,
size: state.size,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ export type CompoundButtonVariants = ButtonVariants;
/**
* {@docCategory Button}
*/
export type CompoundButtonVariantTokens = {
[variant in CompoundButtonVariants]: Partial<CompoundButtonTokens>;
};
export type CompoundButtonVariantTokens = Partial<
{
[variant in CompoundButtonVariants]: Partial<CompoundButtonTokens>;
}
>;
Loading