Skip to content

Commit

Permalink
Slot null rendering implementation (#19273)
Browse files Browse the repository at this point in the history
* Slot null rendering refactor

* Updates react-menu

* Update react-aria

* Updates react-accordion

* Change files

* Stops using assign method

* Updates Input implementation

* Updates react-menu lint error

* Updates API

* Replace optional with required

* Updates resolveshorthand required logic

* Updates react-slider

* Updates react-checkbox

* Change files

* Updates API

* Adds test for required case
  • Loading branch information
bsunderhus authored Aug 18, 2021
1 parent 3431711 commit e467d64
Show file tree
Hide file tree
Showing 34 changed files with 216 additions and 132 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Updates react-accordion on slot null rendering",
"packageName": "@fluentui/react-accordion",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Updates react-aria on slot null rendering",
"packageName": "@fluentui/react-aria",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Updates react-checkbox on slot null rendering",
"packageName": "@fluentui/react-checkbox",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Updates react-menu on slot null rendering",
"packageName": "@fluentui/react-menu",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Slot null rendering refactor",
"packageName": "@fluentui/react-utilities",
"email": "[email protected]",
"dependentChangeType": "patch"
}
2 changes: 1 addition & 1 deletion packages/react-accordion/etc/react-accordion.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export type AccordionHeaderSize = 'small' | 'medium' | 'large' | 'extra-large';
export type AccordionHeaderSlots = {
button: ARIAButtonProps;
expandIcon: AccordionHeaderExpandIconProps;
icon: React_2.HTMLAttributes<HTMLElement>;
icon?: React_2.HTMLAttributes<HTMLElement>;
children: React_2.HTMLAttributes<HTMLElement>;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export type AccordionHeaderSlots = {
/**
* Expand icon slot rendered before (or after) children content in heading
*/
icon: React.HTMLAttributes<HTMLElement>;
icon?: React.HTMLAttributes<HTMLElement>;
children: React.HTMLAttributes<HTMLElement>;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ export const useAccordionHeader = (props: AccordionHeaderProps, ref: React.Ref<H
const innerRef = React.useRef<HTMLElement>(null);

const buttonShorthand = useARIAButton(props.button, {
id,
disabled,
// 'aria-controls': panel?.id,
children: React.Fragment,
required: true,
defaultProps: {
id,
disabled,
// 'aria-controls': panel?.id,
},
});

return {
Expand All @@ -48,7 +50,10 @@ export const useAccordionHeader = (props: AccordionHeaderProps, ref: React.Ref<H
},
icon: resolveShorthand(props.icon),
expandIcon: resolveShorthand(props.expandIcon, {
'aria-hidden': true,
required: true,
defaultProps: {
'aria-hidden': true,
},
}),
button: {
...buttonShorthand,
Expand All @@ -59,6 +64,6 @@ export const useAccordionHeader = (props: AccordionHeaderProps, ref: React.Ref<H
}
}),
},
children: resolveShorthand(props.children),
children: resolveShorthand(props.children, { required: true }),
};
};
2 changes: 1 addition & 1 deletion packages/react-accordion/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"target": "es5",
"lib": ["es5", "dom"],
"lib": ["ES2015", "DOM"],
"outDir": "dist",
"jsx": "react",
"declaration": true,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-aria/etc/react-aria.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { ObjectShorthandProps } from '@fluentui/react-utilities';
import * as React_2 from 'react';
import { ResolveShorthandOptions } from '@fluentui/react-utilities';
import { ShorthandProps } from '@fluentui/react-utilities';

// @public (undocumented)
Expand All @@ -27,8 +28,7 @@ export type ARIAButtonAsElementProps = React_2.HTMLAttributes<HTMLElement> & {
export type ARIAButtonProps = ARIAButtonAsButtonProps | ARIAButtonAsElementProps | ARIAButtonAsAnchorProps;

// @public
export function useARIAButton(value: ShorthandProps<ARIAButtonProps>, defaultProps?: ARIAButtonProps): ObjectShorthandProps<ARIAButtonProps>;

export function useARIAButton<Required extends boolean = false>(value: ShorthandProps<ARIAButtonProps>, options?: ResolveShorthandOptions<ARIAButtonProps, Required>): Required extends false ? ObjectShorthandProps<ARIAButtonProps> | undefined : ObjectShorthandProps<ARIAButtonProps>;

// (No @packageDocumentation comment for this package)

Expand Down
21 changes: 12 additions & 9 deletions packages/react-aria/src/hooks/useARIAButton.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface DefaultArgs {
export const Default = (args: DefaultArgs) => {
const state: State = {
button: {
...useARIAButton({ as: 'button', onClick: args.onClick }),
...useARIAButton({ as: 'button', onClick: args.onClick }, { required: true }),
children: React.Fragment,
},
};
Expand All @@ -28,13 +28,16 @@ export const Default = (args: DefaultArgs) => {
};

export const Anchor = (args: DefaultArgs) => {
const props = useARIAButton({
as: 'a',
onClick: ev => {
ev.preventDefault();
args.onClick(ev);
const props = useARIAButton(
{
as: 'a',
onClick: ev => {
ev.preventDefault();
args.onClick(ev);
},
},
});
{ required: true },
);
const { slots, slotProps } = getSlots(props, []);
return (
<slots.root href="/" {...slotProps.root}>
Expand All @@ -44,13 +47,13 @@ export const Anchor = (args: DefaultArgs) => {
};

export const Span = (args: DefaultArgs) => {
const props = useARIAButton({ as: 'span', onClick: args.onClick });
const props = useARIAButton({ as: 'span', onClick: args.onClick }, { required: true });
const { slots, slotProps } = getSlots(props, []);
return <slots.root {...slotProps.root}>this is a span</slots.root>;
};

export const Div = (args: DefaultArgs) => {
const props = useARIAButton({ as: 'div', onClick: args.onClick });
const props = useARIAButton({ as: 'div', onClick: args.onClick }, { required: true });
const { slots, slotProps } = getSlots(props, []);
return <slots.root {...slotProps.root}>this is a div</slots.root>;
};
Expand Down
18 changes: 12 additions & 6 deletions packages/react-aria/src/hooks/useARIAButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ describe('useARIAButton', () => {

it('should emit click events on Click', () => {
const handleClick = jest.fn();
const { result } = renderHook(() => useARIAButton({ as: 'div', onClick: handleClick }));
const { result } = renderHook(() => useARIAButton({ as: 'div', onClick: handleClick }, { required: true }));
const { slots, slotProps } = getSlots(result.current, []);
render(<slots.root data-testid="div" {...slotProps.root} />);
fireEvent.click(screen.getByTestId('div'));
Expand All @@ -52,7 +52,7 @@ describe('useARIAButton', () => {

it('should emit click events on SpaceBar', () => {
const handleClick = jest.fn();
const { result } = renderHook(() => useARIAButton({ as: 'div', onClick: handleClick }));
const { result } = renderHook(() => useARIAButton({ as: 'div', onClick: handleClick }, { required: true }));
const { slots, slotProps } = getSlots(result.current, []);
render(<slots.root data-testid="div" {...slotProps.root} />);
fireEvent.keyUp(screen.getByTestId('div'), { key: Space });
Expand All @@ -61,7 +61,7 @@ describe('useARIAButton', () => {

it('should emit click events on Enter', () => {
const handleClick = jest.fn();
const { result } = renderHook(() => useARIAButton({ as: 'div', onClick: handleClick }));
const { result } = renderHook(() => useARIAButton({ as: 'div', onClick: handleClick }, { required: true }));
const { slots, slotProps } = getSlots(result.current, []);
render(<slots.root data-testid="div" {...slotProps.root} />);
fireEvent.keyDown(screen.getByTestId('div'), { key: Enter });
Expand All @@ -70,7 +70,9 @@ describe('useARIAButton', () => {

it('should prevent default and stop propagation on disabled while clicking', () => {
const handleClick = jest.fn();
const { result } = renderHook(() => useARIAButton({ as: 'div', 'aria-disabled': true, onClick: handleClick }));
const { result } = renderHook(() =>
useARIAButton({ as: 'div', 'aria-disabled': true, onClick: handleClick }, { required: true }),
);
const { slots, slotProps } = getSlots(result.current, []);
render(
<div onClick={handleClick}>
Expand All @@ -83,7 +85,9 @@ describe('useARIAButton', () => {

it('should prevent default and stop propagation on disabled while pressing SpaceBar', () => {
const handleClick = jest.fn();
const { result } = renderHook(() => useARIAButton({ as: 'div', 'aria-disabled': true, onClick: handleClick }));
const { result } = renderHook(() =>
useARIAButton({ as: 'div', 'aria-disabled': true, onClick: handleClick }, { required: true }),
);
const { slots, slotProps } = getSlots(result.current, []);
render(
<div onClick={handleClick}>
Expand All @@ -96,7 +100,9 @@ describe('useARIAButton', () => {

it('should prevent default and stop propagation on disabled while pressing Enter', () => {
const handleClick = jest.fn();
const { result } = renderHook(() => useARIAButton({ as: 'div', 'aria-disabled': true, onClick: handleClick }));
const { result } = renderHook(() =>
useARIAButton({ as: 'div', 'aria-disabled': true, onClick: handleClick }, { required: true }),
);
const { slots, slotProps } = getSlots(result.current, []);
render(
<div onClick={handleClick}>
Expand Down
58 changes: 35 additions & 23 deletions packages/react-aria/src/hooks/useARIAButton.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import * as React from 'react';
import { ObjectShorthandProps, resolveShorthand, ShorthandProps, useEventCallback } from '@fluentui/react-utilities';
import { Enter, Space } from '@fluentui/keyboard-keys';
import {
ObjectShorthandProps,
resolveShorthand,
ResolveShorthandOptions,
ShorthandProps,
useEventCallback,
} from '@fluentui/react-utilities';

function mergeARIADisabled(disabled?: boolean | 'false' | 'true'): boolean {
if (typeof disabled === 'string') {
Expand All @@ -20,15 +26,18 @@ export type ARIAButtonProps = ARIAButtonAsButtonProps | ARIAButtonAsElementProps
* for multiple scenarios of shorthand properties. Ensuring 1st rule of ARIA for cases
* where no attribute addition is required
*/
export function useARIAButton(
export function useARIAButton<Required extends boolean = false>(
value: ShorthandProps<ARIAButtonProps>,
defaultProps?: ARIAButtonProps,
): ObjectShorthandProps<ARIAButtonProps> {
const shorthand = resolveShorthand(value, defaultProps);
options?: ResolveShorthandOptions<ARIAButtonProps, Required>,
): Required extends false ? ObjectShorthandProps<ARIAButtonProps> | undefined : ObjectShorthandProps<ARIAButtonProps> {
const shorthand = resolveShorthand(value, options);

const { onClick, onKeyDown, onKeyUp, ['aria-disabled']: ariaDisabled } = shorthand;
const { onClick, onKeyDown, onKeyUp, ['aria-disabled']: ariaDisabled } = (shorthand ||
{}) as ObjectShorthandProps<ARIAButtonProps>;

const disabled = mergeARIADisabled((shorthand.as === 'button' ? shorthand.disabled : undefined) ?? ariaDisabled);
const disabled = mergeARIADisabled(
(shorthand && shorthand.as === 'button' ? shorthand.disabled : undefined) ?? ariaDisabled,
);

const onClickHandler: ARIAButtonProps['onClick'] = useEventCallback(ev => {
if (disabled) {
Expand Down Expand Up @@ -82,26 +91,29 @@ export function useARIAButton(
}
});

if (shorthand.as === 'button' || shorthand.as === undefined) {
return shorthand; // there's nothing to be done if as prop === 'button' or undefined
}
if (shorthand) {
if (shorthand.as === 'button' || shorthand.as === undefined) {
return shorthand; // there's nothing to be done if as prop === 'button' or undefined
}

if (!shorthand.hasOwnProperty('role')) {
shorthand.role = 'button';
}
if (!shorthand.hasOwnProperty('aria-disabled')) {
shorthand['aria-disabled'] = disabled;
}
if (!shorthand.hasOwnProperty('role')) {
shorthand.role = 'button';
}
if (!shorthand.hasOwnProperty('aria-disabled')) {
shorthand['aria-disabled'] = disabled;
}

shorthand.onClick = onClickHandler;
shorthand.onKeyDown = onKeyDownHandler;
shorthand.onKeyUp = onKeyupHandler;
shorthand.onClick = onClickHandler;
shorthand.onKeyDown = onKeyDownHandler;
shorthand.onKeyUp = onKeyupHandler;

// Add keydown event handler for all other non-anchor elements.
if (shorthand.as !== 'a') {
if (!shorthand.hasOwnProperty('tabIndex')) {
shorthand.tabIndex = disabled ? undefined : 0;
// Add keydown event handler for all other non-anchor elements.
if (shorthand.as !== 'a') {
if (!shorthand.hasOwnProperty('tabIndex')) {
shorthand.tabIndex = disabled ? undefined : 0;
}
}
}

return shorthand;
}
2 changes: 1 addition & 1 deletion packages/react-aria/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"target": "es5",
"lib": ["es5", "dom"],
"lib": ["ES2015", "DOM"],
"outDir": "dist",
"jsx": "react",
"declaration": true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@ export const useCheckbox = (props: CheckboxProps, ref: React.Ref<HTMLElement>):
},

input: resolveShorthand(props.input, {
type: 'checkbox',
children: null,
required: true,
defaultProps: {
type: 'checkbox',
children: null,
},
}),
indicator: resolveShorthand(props.indicator),
indicator: resolveShorthand(props.indicator, { required: true }),
};

const [checked, setCheckedInternal] = useControllableState({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ export const useCheckboxStyles = (state: CheckboxState): CheckboxState => {
containerStyles[state.size],
!!state.children && containerStyles[state.labelPosition],
);

state.indicator.className = mergeClasses(
indicatorStyles.box,
containerStyles[state.size],
Expand Down
8 changes: 4 additions & 4 deletions packages/react-input/etc/react-input.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ export const inputShorthandProps: (keyof InputSlots)[];
export type InputSlots = {
input: React_2.InputHTMLAttributes<HTMLInputElement>;
inputWrapper: React_2.HTMLAttributes<HTMLElement>;
bookendBefore: React_2.HTMLAttributes<HTMLElement>;
bookendAfter: React_2.HTMLAttributes<HTMLElement>;
insideStart: React_2.HTMLAttributes<HTMLElement>;
insideEnd: React_2.HTMLAttributes<HTMLElement>;
bookendBefore?: React_2.HTMLAttributes<HTMLElement>;
bookendAfter?: React_2.HTMLAttributes<HTMLElement>;
insideStart?: React_2.HTMLAttributes<HTMLElement>;
insideEnd?: React_2.HTMLAttributes<HTMLElement>;
};

// @public
Expand Down
8 changes: 4 additions & 4 deletions packages/react-input/src/components/Input/Input.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ export type InputSlots = {
*/
inputWrapper: React.HTMLAttributes<HTMLElement>;
/** Element before the input field, visually separated from it */
bookendBefore: React.HTMLAttributes<HTMLElement>;
bookendBefore?: React.HTMLAttributes<HTMLElement>;
/** Element after the input field, visually separated from it */
bookendAfter: React.HTMLAttributes<HTMLElement>;
bookendAfter?: React.HTMLAttributes<HTMLElement>;
/** Element at the start of the input field, visually appearing to be inside of it */
insideStart: React.HTMLAttributes<HTMLElement>;
insideStart?: React.HTMLAttributes<HTMLElement>;
/** Element at the end of the input field, visually appearing to be inside of it */
insideEnd: React.HTMLAttributes<HTMLElement>;
insideEnd?: React.HTMLAttributes<HTMLElement>;
};

export interface InputCommons extends Omit<React.HTMLAttributes<HTMLElement>, 'children'> {}
Expand Down
2 changes: 0 additions & 2 deletions packages/react-input/src/components/Input/renderInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import { inputShorthandProps } from './useInput';
*/
export const renderInput = (state: InputState) => {
const { slots, slotProps } = getSlots<InputSlots>(state, inputShorthandProps);
delete slotProps.input.children;
delete slotProps.inputWrapper.children;
return (
<slots.root {...slotProps.root}>
<slots.bookendBefore {...slotProps.bookendBefore} />
Expand Down
Loading

0 comments on commit e467d64

Please sign in to comment.