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

Jump to bottom

MenuButton: Removing legacy patterns from converged component #18498

Merged
merged 3 commits into from
Jun 10, 2021
Merged

MenuButton: Removing legacy patterns from converged component #18498

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16f29d9
MenuButton: Removing legacy patterns from converged component.
khmakoto Jun 9, 2021
0134e2e
Change files
khmakoto Jun 9, 2021
e9560f6
Calling useButtonStyles at the end of useMenuButtonStyles function.
khmakoto Jun 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@fluentui-react-button-5d563384-02a5-4c36-aa04-a80bd9d271b8.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "MenuButton: Removing legacy patterns from converged component.",
"packageName": "@fluentui/react-button",
"email": "Humberto.Morimoto@microsoft.com",
"dependentChangeType": "patch"
}
32 changes: 9 additions & 23 deletions packages/react-button/etc/react-button.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,37 +149,23 @@ export type CompoundButtonVariantTokens = Partial<{
// @public
export const MenuButton: React_2.FunctionComponent<MenuButtonProps & React_2.RefAttributes<HTMLElement>>;

// @public (undocumented)
export type MenuButtonDefaultedProps = ButtonDefaultedProps | 'menuIcon';

// @public (undocumented)
export type MenuButtonProps = Omit<ButtonProps, 'iconPosition'> & {
menuIcon?: ShorthandProps<React_2.HTMLAttributes<HTMLElement>>;
};

// @public
export const menuButtonShorthandProps: readonly ["children", "icon", "menuIcon"];

// @public (undocumented)
export interface MenuButtonState extends Omit<MenuButtonProps, 'children' | 'icon' | 'size'>, Omit<ButtonState, 'iconPosition'> {
// (undocumented)
menuIcon?: ObjectShorthandProps<React_2.HTMLAttributes<HTMLElement>>;
}

// @public (undocumented)
export type MenuButtonStyleSelectors = ButtonStyleSelectors & {};
export type MenuButtonShorthandProps = ButtonShorthandProps | 'menuIcon';

// @public (undocumented)
export type MenuButtonTokens = ButtonTokens & {
menuIconFontSize?: string;
menuIconHeight?: string;
menuIconWidth?: string;
};

// @public (undocumented)
export type MenuButtonVariants = ButtonVariants;
// @public
export const menuButtonShorthandProps: MenuButtonShorthandProps[];

// @public (undocumented)
export type MenuButtonVariantTokens = Partial<{
[variant in MenuButtonVariants]: Partial<MenuButtonTokens>;
}>;
export interface MenuButtonState extends Omit<ButtonState, 'iconPosition'>, ComponentState<MenuButtonProps, MenuButtonShorthandProps, MenuButtonDefaultedProps> {
}

// @public
const renderButton: (state: ButtonState) => JSX.Element;
Expand Down Expand Up @@ -248,7 +234,7 @@ export const useMenuButton: (props: MenuButtonProps, ref: React_2.Ref<HTMLElemen
export const useMenuButtonState: (state: MenuButtonState) => MenuButtonState;

// @public (undocumented)
export const useMenuButtonStyles: (state: MenuButtonState, selectors: MenuButtonStyleSelectors) => void;
export const useMenuButtonStyles: (state: MenuButtonState) => MenuButtonState;

// @public (undocumented)
export const useToggleButton: (props: ToggleButtonProps, ref: React_2.Ref<HTMLElement>, defaultProps?: ToggleButtonProps | undefined) => ToggleButtonState;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-button/src/components/MenuButton/MenuButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MenuButton } from './MenuButton';
import { isConformant } from '../../common/isConformant';
import { MenuButton } from './MenuButton';

describe('MenuButton', () => {
isConformant({
Expand Down
14 changes: 2 additions & 12 deletions packages/react-button/src/components/MenuButton/MenuButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { ChevronDownIcon } from './DefaultIcons';
import { MenuButtonProps, MenuButtonStyleSelectors } from './MenuButton.types';
import { MenuButtonProps } from './MenuButton.types';
import { renderMenuButton } from './renderMenuButton';
import { useMenuButton } from './useMenuButton';
import { useMenuButtonStyles } from './useMenuButtonStyles';
Expand All @@ -17,17 +17,7 @@ export const MenuButton: React.FunctionComponent<MenuButtonProps & React.RefAttr
menuIcon: { as: ChevronDownIcon },
});

const styleSelectors: MenuButtonStyleSelectors = {
disabled: state.disabled,
// expanded: state.expanded,
iconOnly: state.iconOnly,
primary: state.primary,
size: state.size,
subtle: state.subtle,
transparent: state.transparent,
};

useMenuButtonStyles(state, styleSelectors);
useMenuButtonStyles(state);

return renderMenuButton(state);
});
Expand Down
36 changes: 7 additions & 29 deletions packages/react-button/src/components/MenuButton/MenuButton.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { ShorthandProps, ObjectShorthandProps } from '@fluentui/react-utilities';
import { ButtonProps, ButtonState, ButtonStyleSelectors, ButtonTokens, ButtonVariants } from '../Button/Button.types';
import { ComponentState, ShorthandProps } from '@fluentui/react-utilities';
import { ButtonDefaultedProps, ButtonProps, ButtonShorthandProps, ButtonState } from '../Button/Button.types';

/**
* {@docCategory Button}
Expand Down Expand Up @@ -45,38 +45,16 @@ export type MenuButtonProps = Omit<ButtonProps, 'iconPosition'> & {
/**
* {@docCategory Button}
*/
export interface MenuButtonState
extends Omit<MenuButtonProps, 'children' | 'icon' | 'size'>,
Omit<ButtonState, 'iconPosition'> {
menuIcon?: ObjectShorthandProps<React.HTMLAttributes<HTMLElement>>;
}
export type MenuButtonShorthandProps = ButtonShorthandProps | 'menuIcon';

/**
* {@docCategory Button}
*/
export type MenuButtonStyleSelectors = ButtonStyleSelectors & {
// expanded?: boolean;
};
export type MenuButtonDefaultedProps = ButtonDefaultedProps | 'menuIcon';

/**
* {@docCategory Button}
*/
export type MenuButtonTokens = ButtonTokens & {
menuIconFontSize?: string;
menuIconHeight?: string;
menuIconWidth?: string;
};

/**
* {@docCategory Button}
*/
export type MenuButtonVariants = ButtonVariants;

/**
* {@docCategory Button}
*/
export type MenuButtonVariantTokens = Partial<
{
[variant in MenuButtonVariants]: Partial<MenuButtonTokens>;
}
>;
export interface MenuButtonState
extends Omit<ButtonState, 'iconPosition'>,
ComponentState<MenuButtonProps, MenuButtonShorthandProps, MenuButtonDefaultedProps> {}
12 changes: 7 additions & 5 deletions packages/react-button/src/components/MenuButton/useMenuButton.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
import * as React from 'react';
import { makeMergePropsCompat, resolveShorthandProps } from '@fluentui/react-utilities';
import { MenuButtonProps, MenuButtonState } from './MenuButton.types';
import { makeMergeProps, resolveShorthandProps } from '@fluentui/react-utilities';
import { MenuButtonProps, MenuButtonShorthandProps, MenuButtonState } from './MenuButton.types';
import { useMenuButtonState } from './useMenuButtonState';

/**
* Consts listing which props are shorthand props.
*/
export const menuButtonShorthandProps = ['children', 'icon', 'menuIcon'] as const;
export const menuButtonShorthandProps: MenuButtonShorthandProps[] = ['children', 'icon', 'menuIcon'];

// eslint-disable-next-line deprecation/deprecation
const mergeProps = makeMergePropsCompat({ deepMerge: menuButtonShorthandProps });
const mergeProps = makeMergeProps<MenuButtonState>({ deepMerge: menuButtonShorthandProps });

/**
* Redefine the component factory, reusing button factory.
Expand All @@ -21,8 +20,11 @@ export const useMenuButton = (props: MenuButtonProps, ref: React.Ref<HTMLElement
{
ref,
as: 'button',
// Button slots
icon: { as: 'span' },
// MenuButton slots
menuIcon: { as: 'span' },
// Non-slot props
size: 'medium',
},
defaultProps && resolveShorthandProps(defaultProps, menuButtonShorthandProps),
Expand Down
2 changes: 1 addition & 1 deletion packages/react-button/src/components/MenuButton/useMenuButtonState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { MenuButtonState } from './MenuButton.types';
import { useButtonState } from '../Button/useButtonState';
import { MenuButtonState } from './MenuButton.types';

export const useMenuButtonState = (state: MenuButtonState): MenuButtonState => {
// It behaves like a button.
Expand Down
59 changes: 19 additions & 40 deletions packages/react-button/src/components/MenuButton/useMenuButtonStyles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,52 +1,31 @@
import { mergeClasses, makeStyles } from '@fluentui/react-make-styles';
import { Theme } from '@fluentui/react-theme';
import { useButtonStyles } from '../Button/useButtonStyles';
import { MenuButtonState, MenuButtonStyleSelectors, MenuButtonVariantTokens } from './MenuButton.types';
import { MenuButtonState } from './MenuButton.types';

export const makeMenuButtonTokens = (theme: Theme): MenuButtonVariantTokens => ({
base: {
menuIconFontSize: '20px',
menuIconHeight: '20px',
menuIconWidth: '20px',
const useMenuIconStyles = makeStyles({
small: {
fontSize: '20px',
height: '20px',
width: '20px',
},
large: {
menuIconFontSize: '24px',
menuIconHeight: '24px',
menuIconWidth: '24px',
},
});

const useStyles = makeStyles({
menuIcon: theme => {
const menuButtonTokens = makeMenuButtonTokens(theme);

return {
fontSize: menuButtonTokens.base?.menuIconFontSize,
height: menuButtonTokens.base?.menuIconHeight,
width: menuButtonTokens.base?.menuIconWidth,
};
medium: {
fontSize: '20px',
height: '20px',
width: '20px',
},
menuIconLarge: theme => {
const menuButtonTokens = makeMenuButtonTokens(theme);

return {
fontSize: menuButtonTokens.large?.menuIconFontSize,
height: menuButtonTokens.large?.menuIconHeight,
width: menuButtonTokens.large?.menuIconWidth,
};
large: {
fontSize: '24px',
height: '24px',
width: '24px',
},
});

export const useMenuButtonStyles = (state: MenuButtonState, selectors: MenuButtonStyleSelectors) => {
export const useMenuButtonStyles = (state: MenuButtonState): MenuButtonState => {
useButtonStyles(state);
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to CompoundButton, should useButtonStyles be called at the end of this function so that the styles defined here will override the button styles? (Probably not an issue now, but in the future someone might try to add more styles and get confused why it's not working as expected).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, will make the changes.


const styles = useStyles();
const menuIconStyles = useMenuIconStyles();

state.menuIcon.className = mergeClasses(menuIconStyles[state.size], state.menuIcon.className);

if (state.menuIcon) {
state.menuIcon.className = mergeClasses(
styles.menuIcon,
selectors.size === 'large' && styles.menuIconLarge,
state.menuIcon.className,
);
}
return state;
};