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

Root as slot implementation #19483

Merged
merged 25 commits into from
Aug 31, 2021
Merged

Root as slot implementation #19483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0c86106
Adds suppot for root as slot
bsunderhus Aug 24, 2021
762fb40
Updates react-aria to use root as slot
bsunderhus Aug 24, 2021
32b049b
Updates react-accordion to use root as slot
bsunderhus Aug 24, 2021
d7a6c08
Updates react-menu to use root as slot
bsunderhus Aug 24, 2021
69e24a2
Change files
bsunderhus Aug 24, 2021
321e912
Updates react-input to use root as slot
bsunderhus Aug 24, 2021
f3fd6e0
Fix merge conflicts
bsunderhus Aug 24, 2021
94b2021
Update react-menu API
bsunderhus Aug 24, 2021
4357718
Updates react-aria test
bsunderhus Aug 24, 2021
c2a204f
Update change/@fluentui-react-utilities-7755db0b-2b84-49bb-81db-9d949…
bsunderhus Aug 24, 2021
ff5cee8
Updates Internals
bsunderhus Aug 24, 2021
c9bbbbd
skipAsPropTests on react-input
bsunderhus Aug 24, 2021
43aa214
Disables broken conformance tests
bsunderhus Aug 24, 2021
2d76a00
Adds customMount on tests
bsunderhus Aug 24, 2021
c67a33b
Update packages/react-accordion/src/common/isConformant.ts
bsunderhus Aug 25, 2021
2fa48e2
Updates props and useMenuPopover
bsunderhus Aug 25, 2021
60365bd
Updates react-menu API
bsunderhus Aug 25, 2021
96848b5
Updates react-input API
bsunderhus Aug 25, 2021
a39797a
Updates react-accordion API
bsunderhus Aug 25, 2021
1368fbb
Merge branch 'master' of https://github.com/microsoft/fluentui into r…
layershifter Aug 30, 2021
464f5b5
fix imports, update API files
layershifter Aug 30, 2021
cf4294d
fix "import type", update JSDoc, add tests for "null"
layershifter Aug 30, 2021
eb90145
update API file
layershifter Aug 30, 2021
735ec80
apply PR review comment
layershifter Aug 30, 2021
c063fd7
apply PR review comment
layershifter Aug 30, 2021
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
7 changes: 7 additions & 0 deletions change/@fluentui-react-accordion-6527e468-b89b-4934-a503-b6e619e48530.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Updates react-accordion to use root as slot",
"packageName": "@fluentui/react-accordion",
"email": "[email protected]",
"dependentChangeType": "patch"
}
7 changes: 7 additions & 0 deletions change/@fluentui-react-aria-b1783a71-8bf6-4d4b-9e92-6f79c2fbe3d0.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Updates react-aria to use root as slot",
"packageName": "@fluentui/react-aria",
"email": "[email protected]",
"dependentChangeType": "patch"
}
7 changes: 7 additions & 0 deletions change/@fluentui-react-menu-afe42cc7-a1f2-44a8-9e1e-6e15623b7745.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Updates react-menu to use root as slot",
"packageName": "@fluentui/react-menu",
"email": "[email protected]",
"dependentChangeType": "patch"
}
7 changes: 7 additions & 0 deletions change/@fluentui-react-utilities-7755db0b-2b84-49bb-81db-9d949a3e7c17.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Update implementation to handle 'root' as a slot",
"packageName": "@fluentui/react-utilities",
"email": "[email protected]",
"dependentChangeType": "patch"
}
56 changes: 33 additions & 23 deletions packages/react-accordion/etc/react-accordion.api.md
View file
Open in desktop

Large diffs are not rendered by default.

111 changes: 41 additions & 70 deletions packages/react-accordion/src/Accordion.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,56 +10,49 @@ interface AccordionExampleProps
Pick<AccordionHeaderProps, 'icon' | 'inline' | 'size' | 'expandIconPosition'> {}

export const AccordionExample = ({ icon, inline, size, expandIconPosition, ...props }: AccordionExampleProps) => {
const items = [
<AccordionItem value="1" key="1">
<AccordionHeader
inline={inline}
size={size}
expandIconPosition={expandIconPosition}
icon={icon ? <RocketIcon /> : undefined}
>
Accordion Header 1
</AccordionHeader>
<AccordionPanel>
<button onClick={() => setShuffledItems(shuffle(items))}>shuffle</button>
<div>Accordion Panel 1</div>
</AccordionPanel>
</AccordionItem>,
<AccordionItem value="2" key="2">
<AccordionHeader
inline={inline}
size={size}
expandIconPosition={expandIconPosition}
icon={icon ? <RocketIcon /> : undefined}
>
Accordion Header 2
</AccordionHeader>
<AccordionPanel>
<button onClick={() => setShuffledItems(shuffle(items))}>shuffle</button>
<div>Accordion Panel 2</div>
</AccordionPanel>
</AccordionItem>,
<AccordionItem value="3" key="3">
<AccordionHeader
inline={inline}
size={size}
expandIconPosition={expandIconPosition}
icon={icon ? <RocketIcon /> : undefined}
>
Accordion Header 3
</AccordionHeader>
<AccordionPanel>
<button onClick={() => setShuffledItems(shuffle(items))}>shuffle</button>
<div>Accordion Panel 3</div>
</AccordionPanel>
</AccordionItem>,
];

const [shuffledItems, setShuffledItems] = React.useState(items);

return (
<>
<Accordion {...props}>{shuffledItems}</Accordion>
<Accordion {...props}>
<AccordionItem value="1">
<AccordionHeader
inline={inline}
size={size}
expandIconPosition={expandIconPosition}
icon={icon ? <RocketIcon /> : undefined}
>
Accordion Header 1
</AccordionHeader>
<AccordionPanel>
<div>Accordion Panel 1</div>
</AccordionPanel>
</AccordionItem>
<AccordionItem value="2">
<AccordionHeader
inline={inline}
size={size}
expandIconPosition={expandIconPosition}
icon={icon ? <RocketIcon /> : undefined}
>
Accordion Header 2
</AccordionHeader>
<AccordionPanel>
<div>Accordion Panel 2</div>
</AccordionPanel>
</AccordionItem>
<AccordionItem value="3">
<AccordionHeader
inline={inline}
size={size}
expandIconPosition={expandIconPosition}
icon={icon ? <RocketIcon /> : undefined}
>
Accordion Header 3
</AccordionHeader>
<AccordionPanel>
<div>Accordion Panel 3</div>
</AccordionPanel>
</AccordionItem>
</Accordion>
</>
);
};
Expand All @@ -72,10 +65,6 @@ AccordionExample.argTypes = {
defaultValue: false,
control: 'boolean',
},
circular: {
defaultValue: false,
control: 'boolean',
},
multiple: {
defaultValue: false,
control: 'boolean',
Expand Down Expand Up @@ -117,21 +106,3 @@ export default {
title: 'Components/Accordion',
component: Accordion,
};

function shuffle<T>(array: T[]) {
let currentIndex = array.length;
let randomIndex: number;
const nextArray: T[] = Array.from(array);

// While there remain elements to shuffle...
while (currentIndex !== 0) {
// Pick a remaining element...
randomIndex = Math.floor(Math.random() * currentIndex);
currentIndex--;

// And swap it with the current element.
[nextArray[currentIndex], nextArray[randomIndex]] = [nextArray[randomIndex], nextArray[currentIndex]];
}

return nextArray;
}
1 change: 1 addition & 0 deletions packages/react-accordion/src/common/isConformant.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export function isConformant<TProps = {}>(
const defaultOptions: Partial<IsConformantOptions<TProps>> = {
asPropHandlesRef: true,
componentPath: module!.parent!.filename.replace('.test', ''),
skipAsPropTests: true,
Copy link
Member

Choose a reason for hiding this comment

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

Can you please clarify why it is required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The as prop test should be reformulated. it's testing inappropriate cases such as Function component, which is not supported anymore.

Copy link
Member

Choose a reason for hiding this comment

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

Can you please create an issue to follow up?

Copy link
Member

Choose a reason for hiding this comment

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

Please tag me in any follow-up issue for as prop tests too.

};

baseIsConformant(defaultOptions, testInfo);
Expand Down
17 changes: 8 additions & 9 deletions packages/react-accordion/src/components/Accordion/Accordion.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import * as React from 'react';
import type { ComponentProps, ComponentState } from '@fluentui/react-utilities';
import type { ComponentProps, ComponentState, ObjectShorthandProps } from '@fluentui/react-utilities';
import type { AccordionItemValue } from '../AccordionItem/AccordionItem.types';

export type AccordionIndex = number | number[];

export type AccordionToggleEvent<E = HTMLElement> = React.MouseEvent<E> | React.KeyboardEvent<E>;

export type AccordionToggleEventHandler = (event: AccordionToggleEvent, data: AccordionToggleData) => void;
Expand All @@ -22,9 +24,11 @@ export interface AccordionContextValues {
accordion: AccordionContextValue;
}

export type AccordionSlots = {};
export type AccordionSlots = {
root: ObjectShorthandProps<React.HTMLAttributes<HTMLElement>, HTMLElement>;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't root be handled by some common type, to ensure consistent implementation and naming?

};

export interface AccordionCommons extends React.HTMLAttributes<HTMLElement> {
export interface AccordionCommons {
/**
* Indicates if keyboard navigation is available
*/
Expand Down Expand Up @@ -55,9 +59,4 @@ export interface AccordionProps extends ComponentProps<AccordionSlots>, Partial<
onToggle?: AccordionToggleEventHandler;
}

export interface AccordionState extends ComponentState<AccordionSlots>, AccordionCommons, AccordionContextValue {
/**
* Ref to the root slot
*/
ref: React.Ref<HTMLElement>;
}
export interface AccordionState extends ComponentState<AccordionSlots>, AccordionCommons, AccordionContextValue {}
5 changes: 3 additions & 2 deletions packages/react-accordion/src/components/Accordion/renderAccordion.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getSlots } from '@fluentui/react-utilities';
import * as React from 'react';
import { getSlots } from '@fluentui/react-utilities';

import { AccordionContext } from './AccordionContext';
import type { AccordionState, AccordionSlots, AccordionContextValues } from './Accordion.types';

Expand All @@ -11,7 +12,7 @@ export const renderAccordion = (state: AccordionState, contextValues: AccordionC

return (
<slots.root {...slotProps.root}>
<AccordionContext.Provider value={contextValues.accordion}>{state.children}</AccordionContext.Provider>
<AccordionContext.Provider value={contextValues.accordion}>{slotProps.root.children}</AccordionContext.Provider>
</slots.root>
);
};
27 changes: 17 additions & 10 deletions packages/react-accordion/src/components/Accordion/useAccordion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,25 @@
import * as React from 'react';
import { useControllableState, useEventCallback } from '@fluentui/react-utilities';
import type { AccordionProps, AccordionState, AccordionToggleData, AccordionToggleEvent } from './Accordion.types';
import { getNativeElementProps, useControllableState, useEventCallback } from '@fluentui/react-utilities';
import type {
AccordionProps,
AccordionSlots,
AccordionState,
AccordionToggleData,
AccordionToggleEvent,
} from './Accordion.types';
import type { AccordionItemValue } from '../AccordionItem/AccordionItem.types';

export const useAccordion = (
{
export const accordionShorthandProps: Array<keyof AccordionSlots> = ['root'];

export const useAccordion = (props: AccordionProps, ref: React.Ref<HTMLElement>): AccordionState => {
const {
openItems: controlledOpenItems,
defaultOpenItems,
multiple = false,
collapsible = false,
onToggle,
navigable = false,
...rest
}: AccordionProps,
ref: React.Ref<HTMLElement>,
): AccordionState => {
} = props;
const [openItems, setOpenItems] = useControllableState({
state: React.useMemo(() => normalizeValues(controlledOpenItems), [controlledOpenItems]),
defaultState: () => initializeUncontrolledOpenItems({ collapsible, defaultOpenItems, multiple }),
Expand All @@ -32,13 +37,15 @@ export const useAccordion = (
});

return {
...rest,
ref,
multiple,
collapsible,
navigable,
openItems,
requestToggle,
root: getNativeElementProps('div', {
...props,
ref,
}),
};
};

Expand Down
24 changes: 9 additions & 15 deletions packages/react-accordion/src/components/AccordionHeader/AccordionHeader.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as React from 'react';
import type { ComponentProps, ComponentState } from '@fluentui/react-utilities';
import type { ComponentProps, ComponentState, ObjectShorthandProps } from '@fluentui/react-utilities';
import type { AccordionHeaderExpandIconProps } from './AccordionHeaderExpandIcon';
import type { ARIAButtonProps } from '@fluentui/react-aria';
import type { ARIAButtonShorthandProps } from '@fluentui/react-aria';

export type AccordionHeaderSize = 'small' | 'medium' | 'large' | 'extra-large';
export type AccordionHeaderExpandIconPosition = 'start' | 'end';
Expand All @@ -18,22 +18,23 @@ export interface AccordionHeaderContextValues {
}

export type AccordionHeaderSlots = {
root: ObjectShorthandProps<React.HTMLAttributes<HTMLElement>, HTMLElement>;
/**
* The component to be used as button in heading
*/
button: ARIAButtonProps;
button: ARIAButtonShorthandProps;
/**
* Expand icon slot rendered before (or after) children content in heading
*/
expandIcon: AccordionHeaderExpandIconProps;
/**
* Expand icon slot rendered before (or after) children content in heading
*/
icon?: React.HTMLAttributes<HTMLElement>;
children: React.HTMLAttributes<HTMLElement>;
icon?: ObjectShorthandProps<React.HTMLAttributes<HTMLElement>, HTMLElement>;
children: ObjectShorthandProps<React.HTMLAttributes<HTMLElement>, HTMLElement>;
};

export interface AccordionHeaderCommons extends Omit<React.HTMLAttributes<HTMLElement>, 'children'> {
export interface AccordionHeaderCommons {
/**
* Size of spacing in the heading
*/
Expand All @@ -48,16 +49,9 @@ export interface AccordionHeaderCommons extends Omit<React.HTMLAttributes<HTMLEl
inline: boolean;
}

export interface AccordionHeaderProps
extends ComponentProps<Partial<AccordionHeaderSlots>>,
Partial<AccordionHeaderCommons> {}
export interface AccordionHeaderProps extends ComponentProps<AccordionHeaderSlots>, Partial<AccordionHeaderCommons> {}

export interface AccordionHeaderState
extends ComponentState<AccordionHeaderSlots>,
AccordionHeaderCommons,
AccordionHeaderContextValue {
/**
* Ref to the root slot
*/
ref: React.Ref<HTMLElement>;
}
AccordionHeaderContextValue {}
6 changes: 5 additions & 1 deletion packages/react-accordion/src/components/AccordionHeader/AccordionHeaderExpandIcon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import * as React from 'react';
import { useAccordionHeaderContext } from './AccordionHeaderContext';
import type { AccordionHeaderContextValue } from './AccordionHeader.types';
import type { ObjectShorthandProps } from '@fluentui/react-utilities';

export type AccordionHeaderExpandIconProps = React.HTMLAttributes<HTMLSpanElement>;
export type AccordionHeaderExpandIconProps = ObjectShorthandProps<
React.HTMLAttributes<HTMLSpanElement>,
HTMLSpanElement
>;

export const AccordionHeaderExpandIcon = React.forwardRef<HTMLSpanElement, AccordionHeaderExpandIconProps>(
({ children, ...rest }, ref) => {
Expand Down
1 change: 0 additions & 1 deletion ...eact-accordion/src/components/AccordionHeader/__snapshots__/AccordionHeader.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ exports[`AccordionHeader renders a default state 1`] = `
<button
className=""
disabled={false}
id="accordion-header-1"
Copy link
Member

Choose a reason for hiding this comment

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

Is it expected that we don't have id there anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. this is due to removal of id mechanism. Has definitely nothing to do with this PR, I'll try to remove this change

onClick={[Function]}
>
<span
Expand Down
Loading