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

Button: Creating ActionButton variant and cleaning styling #8645

Merged
merged 4 commits into from
Apr 10, 2019
Merged

Button: Creating ActionButton variant and cleaning styling #8645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0110e72
Button: Creating ActionButton variant and cleaning styling.
khmakoto Apr 8, 2019
19a0e17
Adding change file.
khmakoto Apr 8, 2019
cca2e83
Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…
khmakoto Apr 8, 2019
b6e57de
Fixing padding.
khmakoto Apr 9, 2019
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
11 changes: 11 additions & 0 deletions common/changes/@uifabric/experiments/actionButton_2019-04-08-21-50.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@uifabric/experiments",
"comment": "Button: Creating ActionButton variant and cleaning styling.",
"type": "minor"
}
],
"packageName": "@uifabric/experiments",
"email": "[email protected]"
}
49 changes: 49 additions & 0 deletions packages/experiments/src/components/Button/ActionButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import * as React from 'react';
import { Button } from './Button';
import { IButtonComponent, IButtonTokenReturnType } from './Button.types';
import { ButtonVariantsType } from './ButtonVariants.types';
import { FontWeights } from '../../Styling';

const baseTokens: IButtonComponent['tokens'] = (props, theme): IButtonTokenReturnType => {
const { palette } = theme;

return {
backgroundColor: 'transparent',
backgroundColorHovered: 'transparent',
backgroundColorPressed: 'transparent',
borderColor: 'transparent',
color: palette.neutralPrimary,
colorHovered: palette.themePrimary,
colorPressed: palette.black,
contentPadding: '0px 8px',
height: '40px',
iconColor: palette.neutralPrimary,
iconColorHovered: palette.themePrimary,
iconColorPressed: palette.black,
textWeight: FontWeights.regular
};
};

const disabledTokens: IButtonComponent['tokens'] = (props, theme): IButtonTokenReturnType => {
const { palette } = theme;

return {
color: palette.neutralTertiary,
colorHovered: palette.neutralTertiary,
colorPressed: palette.neutralTertiary,
iconColor: palette.neutralTertiary,
iconColorHovered: palette.neutralTertiary,
iconColorPressed: palette.neutralTertiary
};
};

export const ActionButtonTokens: IButtonComponent['tokens'] = (props, theme): IButtonTokenReturnType => [
baseTokens,
props.disabled && disabledTokens
];

export const ActionButton: ButtonVariantsType = props => {
const { text, iconProps, ...rest } = props;

return <Button stack={{ horizontalAlign: 'start' }} content={text} icon={iconProps} tokens={ActionButtonTokens} {...rest} />;
};
7 changes: 5 additions & 2 deletions packages/experiments/src/components/Button/Button.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { IsFocusVisibleClassName } from '../../Utilities';
const baseTokens: IButtonComponent['tokens'] = {
borderRadius: 0,
borderWidth: 1,
cursor: 'pointer',
minWidth: 100,
minHeight: 32,
lineHeight: 1,
Expand Down Expand Up @@ -96,7 +97,9 @@ const disabledTokens: IButtonComponent['tokens'] = (props, theme): IButtonTokenR

highContrastBorderColor: 'GrayText',
highContrastBorderColorHovered: 'GrayText',
highContrastBorderColorPressed: 'GrayText'
highContrastBorderColorPressed: 'GrayText',

cursor: 'default'
};
};

Expand Down Expand Up @@ -165,7 +168,7 @@ export const ButtonStyles: IButtonComponent['styles'] = (props, theme, tokens):
borderWidth: tokens.borderWidth,
boxSizing: 'border-box',
color: tokens.color,
cursor: 'default',
cursor: tokens.cursor,
display: 'inline-block',
fontSize: tokens.textSize,
fontWeight: tokens.textWeight,
Expand Down
1 change: 1 addition & 0 deletions packages/experiments/src/components/Button/Button.types.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ export interface IButtonTokens {
borderWidth?: number | string;
contentPadding?: number | string;
contentPaddingFocused?: number | string;
cursor?: string | undefined;
textFamily?: string;
textSize?: number | string;
textWeight?: IFontWeight;
Expand Down
6 changes: 6 additions & 0 deletions packages/experiments/src/components/Button/ButtonPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import { MenuButtonExample } from './MenuButton/examples/MenuButton.Example';
import { SplitButtonExample } from './SplitButton/examples/SplitButton.Example';
import { ButtonStylesExample } from './examples/Button.Styles.Example';
import { ButtonTokensExample } from './examples/Button.Tokens.Example';
import { ButtonVariantsExample } from './examples/Button.Variants.Example';

const ButtonExampleCode = require('!raw-loader!@uifabric/experiments/src/components/Button/examples/Button.Example.tsx') as string;
const MenuButtonExampleCode = require('!raw-loader!@uifabric/experiments/src/components/Button/MenuButton/examples/MenuButton.Example.tsx') as string;
const SplitButtonExampleCode = require('!raw-loader!@uifabric/experiments/src/components/Button/SplitButton/examples/SplitButton.Example.tsx') as string;
const ButtonStylesExampleCode = require('!raw-loader!@uifabric/experiments/src/components/Button/examples/Button.Styles.Example.tsx') as string;
const ButtonTokensExampleCode = require('!raw-loader!@uifabric/experiments/src/components/Button/examples/Button.Tokens.Example.tsx') as string;
const ButtonVariantsExampleCode = require('!raw-loader!@uifabric/experiments/src/components/Button/examples/Button.Tokens.Example.tsx') as string;

export class ButtonPage extends React.Component<IComponentDemoPageProps, {}> {
public render(): JSX.Element {
Expand All @@ -30,6 +32,9 @@ export class ButtonPage extends React.Component<IComponentDemoPageProps, {}> {
<ExampleCard title="Split Button with two focus stops" code={SplitButtonExampleCode}>
<SplitButtonExample />
</ExampleCard>
<ExampleCard title="Button Variants Examples" code={ButtonVariantsExampleCode}>
<ButtonVariantsExample />
</ExampleCard>
<ExampleCard title="Button Styles" code={ButtonStylesExampleCode}>
<ButtonStylesExample />
</ExampleCard>
Expand All @@ -42,6 +47,7 @@ export class ButtonPage extends React.Component<IComponentDemoPageProps, {}> {
<PropertiesTableSet
sources={[
require<string>('!raw-loader!@uifabric/experiments/src/components/Button/Button.types.tsx'),
require<string>('!raw-loader!@uifabric/experiments/src/components/Button/ButtonVariants.types.ts'),
require<string>('!raw-loader!@uifabric/experiments/src/components/Button/MenuButton/MenuButton.types.tsx'),
require<string>('!raw-loader!@uifabric/experiments/src/components/Button/SplitButton/SplitButton.types.tsx')
]}
Expand Down
2 changes: 2 additions & 0 deletions packages/experiments/src/components/Button/ButtonVariants.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { IButtonProps } from './Button.types';
import { IIconProps } from 'office-ui-fabric-react';

export interface IButtonVariantProps extends IButtonProps {
iconProps?: IIconProps;
Copy link
Member

Choose a reason for hiding this comment

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

I think we should consider marking this prop as deprecated in master and then remove it for Fabric 7 release, which would mean we wouldn't need to add it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

But how can we mark it as deprecated in the current Button in oufr if we still don't offer an alternative in that production component?

Copy link
Member

Choose a reason for hiding this comment

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

By adding a content prop.. should be relatively easy. Unless we don't think that's a good permanent name for Button content, in which case we have a larger problem to address.

Copy link
Member Author

Choose a reason for hiding this comment

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

And then we do the same for iconProps? Deprecate it in favor of a new icon prop?

Copy link
Member

Choose a reason for hiding this comment

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

Possibly. It's either that or we carry over duplicate names for props even in Fabric 7 which seems cumbersome to take on for all of 7.x. I wonder what @dzearing thinks.

text?: string;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/experiments/src/components/Button/DefaultButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Button } from './Button';
import { ButtonVariantsType } from './ButtonVariants.types';

export const DefaultButton: ButtonVariantsType = props => {
const { text, ...rest } = props;
const { text, iconProps, ...rest } = props;
Copy link
Member

Choose a reason for hiding this comment

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

I didn't catch this in the other PR but maybe text is another prop that could be deprecated in master and removed in Fabric 7.


return <Button content={text} {...rest} />;
return <Button content={text} icon={iconProps} {...rest} />;
};
4 changes: 2 additions & 2 deletions packages/experiments/src/components/Button/PrimaryButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Button } from './Button';
import { ButtonVariantsType } from './ButtonVariants.types';

export const PrimaryButton: ButtonVariantsType = props => {
const { text, ...rest } = props;
const { text, iconProps, ...rest } = props;

return <Button primary content={text} {...rest} />;
return <Button primary content={text} icon={iconProps} {...rest} />;
Copy link
Member

Choose a reason for hiding this comment

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

What is the name that appears in the hierarchy for these variants? If it's Button then styling and theming targeting PrimaryButton by name won't work. If both Button and PrimaryButton are appearing in hierarchy, then I think styling and theming will be even more wonky.

All of this is just driving need for an extendComponent utility that will just have one entry called PrimaryButton in the hierarchy that should still take directed styling and theming. This is covered by #8331 (and it seems should be resolved before Fabric 7 release.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think only Button appears in the hierarchy right now.

Copy link
Member

@JasonGore JasonGore Apr 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The other button variants appear as Unknown. We could apply displayName to them but that won't fix the styling issue since there is no helper like styled reading the setting and applying it (the variants are just thin wrappers.)

I'll add a note to #8331. The only other solution I can think of is to create new variants using styled or createComponent.

};
7 changes: 3 additions & 4 deletions packages/experiments/src/components/Button/SplitButton/SplitButton.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,10 @@ export const SplitButtonStyles: ISplitButtonComponent['styles'] = (props, theme,
}
},
splitDivider: {
borderRight: '1px solid',
borderColor: tokens.color,
backgroundColor: tokens.color,
boxSizing: 'border-box',
height: 'calc(100% - 16px)',
margin: '8px 0px',
height: 'calc(100% - 14px)',
margin: '7px 0px',
width: 1,

selectors: {
Expand Down
4 changes: 2 additions & 2 deletions packages/experiments/src/components/Button/__snapshots__/Button.view.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ exports[`Button view renders a default Button with content correctly 1`] = `
border-width: 1px;
box-sizing: border-box;
color: #333333;
cursor: default;
cursor: pointer;
display: inline-block;
font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
font-size: 14px;
Expand Down Expand Up @@ -293,7 +293,7 @@ exports[`Button view renders a primary Button with content correctly 1`] = `
border-width: 1px;
box-sizing: border-box;
color: #ffffff;
cursor: default;
cursor: pointer;
display: inline-block;
font-family: 'Segoe UI', 'Segoe UI Web (West European)', 'Segoe UI', -apple-system, BlinkMacSystemFont, 'Roboto', 'Helvetica Neue', sans-serif;
font-size: 14px;
Expand Down
44 changes: 44 additions & 0 deletions packages/experiments/src/components/Button/examples/Button.Variants.Example.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import * as React from 'react';
import { ActionButton, DefaultButton, PrimaryButton } from '../index';
import { Stack } from 'office-ui-fabric-react';

const tokens = {
sectionStack: {
childrenGap: 32
},
buttonStack: {
childrenGap: 12
}
};

const alertClicked = (): void => {
alert('Clicked');
};

const ButtonStack = (props: { children: JSX.Element[] | JSX.Element }) => (
<Stack horizontal disableShrink tokens={tokens.buttonStack}>
{props.children}
</Stack>
);

// tslint:disable:jsx-no-lambda
export class ButtonVariantsExample extends React.Component<{}, {}> {
public render(): JSX.Element {
return (
<Stack tokens={tokens.sectionStack}>
<Stack tokens={tokens.buttonStack}>
<ButtonStack>
<DefaultButton text="Default button" onClick={alertClicked} />
<DefaultButton disabled text="Disabled default button" onClick={alertClicked} />
<PrimaryButton text="Primary button" onClick={alertClicked} />
<PrimaryButton disabled text="Disabled primary button" onClick={alertClicked} />
</ButtonStack>
<ButtonStack>
<ActionButton text="Action button" onClick={alertClicked} />
<ActionButton disabled text="Disabled action button" onClick={alertClicked} />
</ButtonStack>
</Stack>
</Stack>
);
}
}
1 change: 1 addition & 0 deletions packages/experiments/src/components/Button/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './Button';
export * from './Button.types';
export * from './ButtonVariants.types';
export * from './ActionButton';
export * from './DefaultButton';
export * from './PrimaryButton';