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.

Sign up for GitHub

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

chore: Cleaning up tokens in Button components so they better adhere to the design spec #24732

Merged
merged 7 commits into from
Sep 12, 2022
Merged

chore: Cleaning up tokens in Button components so they better adhere to the design spec #24732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
21439c2
chore: Cleaning up tokens in Button components so they better adhere …
Sep 8, 2022
7980188
Adding change file.
Sep 8, 2022
126356e
Applying latest changes from design docs.
Sep 12, 2022
98a1afb
Fixing HC issues.
Sep 12, 2022
f9f0e6f
Fixing HC issues.
Sep 12, 2022
41bbb00
Fixing HC issues.
Sep 12, 2022
c031146
Adding proper icon to Button vr stories.
Sep 12, 2022
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
86 changes: 49 additions & 37 deletions apps/vr-tests-react-components/src/stories/Button.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ import { storiesOf } from '@storybook/react';
import * as React from 'react';
import Screener from 'screener-storybook/src/screener';
import { Button, CompoundButton, ToggleButton, MenuButton } from '@fluentui/react-button';
import { bundleIcon, CalendarMonthFilled, CalendarMonthRegular } from '@fluentui/react-icons';
import { makeStyles } from '@griffel/react';

const CalendarMonth = bundleIcon(CalendarMonthFilled, CalendarMonthRegular);

const steps = new Screener.Steps()
.snapshot('default', { cropTo: '.testWrapper' })
.hover('#button-id')
Expand Down Expand Up @@ -125,43 +128,43 @@ storiesOf('Button Converged', module)
{ includeHighContrast: true, includeDarkMode: true },
)
.addStory('Size small', () => (
<Button id={buttonId} icon="X" size="small">
<Button id={buttonId} icon={<CalendarMonth />} size="small">
Hello, world
</Button>
))
.addStory('Size large', () => (
<Button id={buttonId} icon="X" size="large">
<Button id={buttonId} icon={<CalendarMonth />} size="large">
Hello, world
</Button>
))
.addStory('Size small - with long text wrapping', () => {
const styles = useStyles();
return (
<Button id={buttonId} className={styles.longText} icon="X" size="small">
<Button id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="small">
Long text wraps after it hits the max width of the component
</Button>
);
})
.addStory('Size medium - with long text wrapping', () => {
const styles = useStyles();
return (
<Button id={buttonId} className={styles.longText} icon="X" size="medium">
<Button id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="medium">
Long text wraps after it hits the max width of the component
</Button>
);
})
.addStory('Size large - with long text wrapping', () => {
const styles = useStyles();
return (
<Button id={buttonId} className={styles.longText} icon="X" size="large">
<Button id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="large">
Long text wraps after it hits the max width of the component
</Button>
);
})
.addStory(
'With icon before content',
() => (
<Button id={buttonId} icon="X">
<Button id={buttonId} icon={<CalendarMonth />}>
Hello, world
</Button>
),
Expand All @@ -172,14 +175,14 @@ storiesOf('Button Converged', module)
.addStory(
'With icon after content',
() => (
<Button id={buttonId} icon="X" iconPosition="after">
<Button id={buttonId} icon={<CalendarMonth />} iconPosition="after">
Hello, world
</Button>
),
{ includeRtl: true },
)
.addStory('Icon only', () => <Button id={buttonId} icon="X" />)
.addStory('Circular and icon only', () => <Button id={buttonId} shape="circular" icon="X" />, {
.addStory('Icon only', () => <Button id={buttonId} icon={<CalendarMonth />} />)
.addStory('Circular and icon only', () => <Button id={buttonId} shape="circular" icon={<CalendarMonth />} />, {
includeRtl: true,
});

Expand Down Expand Up @@ -210,7 +213,7 @@ storiesOf('CompoundButton Converged', module)
.addStory(
'With icon before content',
() => (
<CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon="X">
<CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon={<CalendarMonth />}>
Hello, world
</CompoundButton>
),
Expand All @@ -219,7 +222,12 @@ storiesOf('CompoundButton Converged', module)
.addStory(
'With icon after content',
() => (
<CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon="X" iconPosition="after">
<CompoundButton
id={buttonId}
secondaryContent="This is some secondary text"
icon={<CalendarMonth />}
iconPosition="after"
>
Hello, world
</CompoundButton>
),
Expand Down Expand Up @@ -279,12 +287,12 @@ storiesOf('CompoundButton Converged', module)
</CompoundButton>
))
.addStory('Size small', () => (
<CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon="X" size="small">
<CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon={<CalendarMonth />} size="small">
Hello, world
</CompoundButton>
))
.addStory('Size large', () => (
<CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon="X" size="large">
<CompoundButton id={buttonId} secondaryContent="This is some secondary text" icon={<CalendarMonth />} size="large">
Hello, world
</CompoundButton>
))
Expand All @@ -295,7 +303,7 @@ storiesOf('CompoundButton Converged', module)
id={buttonId}
className={styles.longText}
secondaryContent="This is some secondary text"
icon="X"
icon={<CalendarMonth />}
size="small"
>
Long text wraps after it hits the max width of the component
Expand All @@ -309,7 +317,7 @@ storiesOf('CompoundButton Converged', module)
id={buttonId}
className={styles.longText}
secondaryContent="This is some secondary text"
icon="X"
icon={<CalendarMonth />}
size="medium"
>
Long text wraps after it hits the max width of the component
Expand All @@ -323,17 +331,21 @@ storiesOf('CompoundButton Converged', module)
id={buttonId}
className={styles.longText}
secondaryContent="This is some secondary text"
icon="X"
icon={<CalendarMonth />}
size="large"
>
Long text wraps after it hits the max width of the component
</CompoundButton>
);
})
.addStory('Icon only', () => <CompoundButton id={buttonId} icon="X" />)
.addStory('Circular and icon only', () => <CompoundButton id={buttonId} shape="circular" icon="X" />, {
includeRtl: true,
});
.addStory('Icon only', () => <CompoundButton id={buttonId} icon={<CalendarMonth />} />)
.addStory(
'Circular and icon only',
() => <CompoundButton id={buttonId} shape="circular" icon={<CalendarMonth />} />,
{
includeRtl: true,
},
);

storiesOf('ToggleButton Converged', module)
.addDecorator(story => <Screener steps={steps}>{story()}</Screener>)
Expand Down Expand Up @@ -395,51 +407,51 @@ storiesOf('ToggleButton Converged', module)
</ToggleButton>
))
.addStory('Size small', () => (
<ToggleButton id={buttonId} icon="X" size="small">
<ToggleButton id={buttonId} icon={<CalendarMonth />} size="small">
Hello, world
</ToggleButton>
))
.addStory('Size large', () => (
<ToggleButton id={buttonId} icon="X" size="large">
<ToggleButton id={buttonId} icon={<CalendarMonth />} size="large">
Hello, world
</ToggleButton>
))
.addStory('Size small - with long text wrapping', () => {
const styles = useStyles();
return (
<ToggleButton id={buttonId} className={styles.longText} icon="X" size="small">
<ToggleButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="small">
Long text wraps after it hits the max width of the component
</ToggleButton>
);
})
.addStory('Size medium - with long text wrapping', () => {
const styles = useStyles();
return (
<ToggleButton id={buttonId} className={styles.longText} icon="X" size="medium">
<ToggleButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="medium">
Long text wraps after it hits the max width of the component
</ToggleButton>
);
})
.addStory('Size large - with long text wrapping', () => {
const styles = useStyles();
return (
<ToggleButton id={buttonId} className={styles.longText} icon="X" size="large">
<ToggleButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="large">
Long text wraps after it hits the max width of the component
</ToggleButton>
);
})
.addStory('With icon before content', () => (
<ToggleButton id={buttonId} icon="X">
<ToggleButton id={buttonId} icon={<CalendarMonth />}>
Hello, world
</ToggleButton>
))
.addStory('With icon after content', () => (
<ToggleButton id={buttonId} icon="X" iconPosition="after">
<ToggleButton id={buttonId} icon={<CalendarMonth />} iconPosition="after">
Hello, world
</ToggleButton>
))
.addStory('Icon only', () => <ToggleButton id={buttonId} icon="X" />)
.addStory('Circular and icon only', () => <ToggleButton id={buttonId} shape="circular" icon="X" />)
.addStory('Icon only', () => <ToggleButton id={buttonId} icon={<CalendarMonth />} />)
.addStory('Circular and icon only', () => <ToggleButton id={buttonId} shape="circular" icon={<CalendarMonth />} />)
.addStory('Checked', () => (
<ToggleButton id={buttonId} checked>
Hello, world
Expand Down Expand Up @@ -515,43 +527,43 @@ storiesOf('MenuButton Converged', module)
</MenuButton>
))
.addStory('Size small', () => (
<MenuButton id={buttonId} icon="X" size="small">
<MenuButton id={buttonId} icon={<CalendarMonth />} size="small">
Hello, world
</MenuButton>
))
.addStory('Size large', () => (
<MenuButton id={buttonId} icon="X" size="large">
<MenuButton id={buttonId} icon={<CalendarMonth />} size="large">
Hello, world
</MenuButton>
))
.addStory('Size small - with long text wrapping', () => {
const styles = useStyles();
return (
<MenuButton id={buttonId} className={styles.longText} icon="X" size="small">
<MenuButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="small">
Long text wraps after it hits the max width of the component
</MenuButton>
);
})
.addStory('Size medium - with long text wrapping', () => {
const styles = useStyles();
return (
<MenuButton id={buttonId} className={styles.longText} icon="X" size="medium">
<MenuButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="medium">
Long text wraps after it hits the max width of the component
</MenuButton>
);
})
.addStory('Size large - with long text wrapping', () => {
const styles = useStyles();
return (
<MenuButton id={buttonId} className={styles.longText} icon="X" size="large">
<MenuButton id={buttonId} className={styles.longText} icon={<CalendarMonth />} size="large">
Long text wraps after it hits the max width of the component
</MenuButton>
);
})
.addStory('With icon', () => (
<MenuButton id={buttonId} icon="X">
<MenuButton id={buttonId} icon={<CalendarMonth />}>
Hello, world
</MenuButton>
))
.addStory('Icon only', () => <MenuButton id={buttonId} icon="X" />)
.addStory('Circular and icon only', () => <MenuButton id={buttonId} shape="circular" icon="X" />);
.addStory('Icon only', () => <MenuButton id={buttonId} icon={<CalendarMonth />} />)
.addStory('Circular and icon only', () => <MenuButton id={buttonId} shape="circular" icon={<CalendarMonth />} />);
7 changes: 7 additions & 0 deletions change/@fluentui-react-button-2bff9178-dae7-48c6-9b2e-4e5effb0d484.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "chore: Cleaning up tokens in Button components so they better adhere to the design spec.",
"packageName": "@fluentui/react-button",
"email": "humberto_makoto@hotmail.com",
"dependentChangeType": "patch"
}
36 changes: 24 additions & 12 deletions packages/react-components/react-button/src/components/Button/useButtonStyles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,13 @@ const useRootStyles = makeStyles({
':hover': {
backgroundColor: tokens.colorSubtleBackgroundHover,
...shorthands.borderColor('transparent'),
color: tokens.colorNeutralForeground2BrandHover,
color: tokens.colorNeutralForeground2Hover,
},

':hover:active': {
backgroundColor: tokens.colorSubtleBackgroundPressed,
...shorthands.borderColor('transparent'),
color: tokens.colorNeutralForeground2BrandPressed,
color: tokens.colorNeutralForeground2Pressed,
},
},
transparent: {
Expand Down Expand Up @@ -270,11 +270,11 @@ const useRootDisabledStyles = makeStyles({
backgroundColor: tokens.colorTransparentBackground,

':hover': {
backgroundColor: tokens.colorTransparentBackgroundHover,
backgroundColor: tokens.colorTransparentBackground,
},

':hover:active': {
backgroundColor: tokens.colorTransparentBackgroundPressed,
backgroundColor: tokens.colorTransparentBackground,
},
},
primary: {
Expand All @@ -292,39 +292,39 @@ const useRootDisabledStyles = makeStyles({
/* The secondary styles are exactly the same as the base styles. */
},
subtle: {
backgroundColor: 'transparent',
backgroundColor: tokens.colorTransparentBackground,
...shorthands.borderColor('transparent'),

':hover': {
backgroundColor: 'transparent',
backgroundColor: tokens.colorTransparentBackground,
...shorthands.borderColor('transparent'),
},

':hover:active': {
backgroundColor: 'transparent',
backgroundColor: tokens.colorTransparentBackground,
...shorthands.borderColor('transparent'),
},
},
transparent: {
backgroundColor: 'transparent',
backgroundColor: tokens.colorTransparentBackground,
...shorthands.borderColor('transparent'),

':hover': {
backgroundColor: 'transparent',
backgroundColor: tokens.colorTransparentBackground,
...shorthands.borderColor('transparent'),
},

':hover:active': {
backgroundColor: 'transparent',
backgroundColor: tokens.colorTransparentBackground,
...shorthands.borderColor('transparent'),
},
},
});

const useRootFocusStyles = makeStyles({
base: createCustomFocusIndicatorStyle({
...shorthands.borderColor('transparent'),
outlineColor: 'transparent',
...shorthands.borderColor(tokens.colorTransparentStroke),
outlineColor: tokens.colorTransparentStroke,
outlineWidth: tokens.strokeWidthThick,
outlineStyle: 'solid',
boxShadow: `
Expand Down Expand Up @@ -416,6 +416,17 @@ const useIconStyles = makeStyles({
[iconSpacingVar]: tokens.spacingHorizontalSNudge,
},

// Appearance variations
subtle: {
':hover': {
color: tokens.colorNeutralForeground2BrandHover,
},

':hover:active': {
color: tokens.colorNeutralForeground2BrandPressed,
},
},

// Icon position variations
before: {
marginRight: `var(${iconSpacingVar})`,
Expand Down Expand Up @@ -467,6 +478,7 @@ export const useButtonStyles_unstable = (state: ButtonState): ButtonState => {
buttonClassNames.icon,
iconStyles.base,
state.root.children !== undefined && state.root.children !== null && iconStyles[iconPosition],
appearance === 'subtle' && iconStyles.subtle,
iconStyles[size],
state.icon.className,
);
Expand Down
Loading