-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(react-infobutton): Add initial implementation #25247
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.
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
Changes from 2 commits
29c1a87
eee5466
c9c76d1
3f53fd9
75bc33b
8630172
b225c04
7883f2b
381d61c
2f9aa37
754a7f3
e80518f
ab0e929
668d364
eebe657
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import { Info12Regular, Info12Filled, bundleIcon } from '@fluentui/react-icons'; | ||
|
|
||
| export const DefaultInfoButtonIcon = bundleIcon(Info12Filled, Info12Regular); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,52 @@ | ||
| import * as React from 'react'; | ||
| import { render } from '@testing-library/react'; | ||
| import { InfoButton } from './InfoButton'; | ||
| import { isConformant } from '../../common/isConformant'; | ||
| import { infoButtonClassNames } from './useInfoButtonStyles'; | ||
| import { RenderResult } from '@testing-library/react'; | ||
|
|
||
| // testing-library's queryByRole function doesn't look inside portals | ||
| function queryByRoleDialog(result: RenderResult) { | ||
| const dialogs = result.baseElement.querySelectorAll('*[role="dialog"]'); | ||
|
sopranopillow marked this conversation as resolved.
Outdated
|
||
| if (!dialogs?.length) { | ||
| return null; | ||
| } else { | ||
| expect(dialogs.length).toBe(1); | ||
| return dialogs.item(0) as HTMLElement; | ||
| } | ||
| } | ||
|
|
||
| const getPopoverSurfaceElement = (result: RenderResult) => { | ||
| // triggerButton needs to be clicked otherwise content won't be rendered. | ||
| result.getByRole('button').click(); | ||
| const dialog = queryByRoleDialog(result); | ||
| expect(dialog).not.toBeNull(); | ||
| return dialog!; | ||
| }; | ||
|
|
||
| describe('InfoButton', () => { | ||
| isConformant({ | ||
| Component: InfoButton, | ||
| displayName: 'InfoButton', | ||
| }); | ||
|
|
||
| // TODO add more tests here, and create visual regression tests in /apps/vr-tests | ||
|
|
||
| it('renders a default state', () => { | ||
| const result = render(<InfoButton>Default InfoButton</InfoButton>); | ||
| expect(result.container).toMatchSnapshot(); | ||
| requiredProps: { | ||
| content: 'Popover content', | ||
| }, | ||
| disabledTests: [ | ||
| 'component-handles-ref', | ||
| 'component-has-root-ref', | ||
| 'component-handles-classname', | ||
| 'make-styles-overrides-win', | ||
| ], | ||
| testOptions: { | ||
| 'has-static-classnames': [ | ||
| { | ||
| props: {}, | ||
| expectedClassNames: { | ||
| // root shouldn't be expected since the root is a Popover | ||
| trigger: infoButtonClassNames.trigger, | ||
| content: infoButtonClassNames.content, | ||
| }, | ||
| getPortalElement: getPopoverSurfaceElement, | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,29 @@ | ||
| import type { Button } from '@fluentui/react-button'; | ||
| import type { PopoverProps, PopoverSurface } from '@fluentui/react-popover'; | ||
| import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities'; | ||
|
|
||
| export type InfoButtonSlots = { | ||
| root: Slot<'div'>; | ||
| root: NonNullable<Slot<PopoverProps>>; | ||
|
|
||
| /** | ||
| * The button that triggers the Popover. | ||
| */ | ||
| trigger: NonNullable<Slot<typeof Button>>; | ||
|
|
||
| /** | ||
| * The content to be displayed in the Popover. | ||
| */ | ||
| content: NonNullable<Slot<typeof PopoverSurface>>; | ||
| }; | ||
|
|
||
| /** | ||
| * InfoButton Props | ||
| */ | ||
| export type InfoButtonProps = ComponentProps<InfoButtonSlots> & {}; | ||
| export type InfoButtonProps = Omit<ComponentProps<Partial<InfoButtonSlots>>, 'children'>; | ||
|
sopranopillow marked this conversation as resolved.
Outdated
|
||
|
|
||
| /** | ||
| * State used in rendering InfoButton | ||
| */ | ||
| export type InfoButtonState = ComponentState<InfoButtonSlots>; | ||
| export type InfoButtonState = ComponentState<InfoButtonSlots> & { | ||
| popoverOpen: boolean; | ||
| }; | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,20 @@ | ||
| import * as React from 'react'; | ||
| import { getSlots } from '@fluentui/react-utilities'; | ||
| import type { InfoButtonState, InfoButtonSlots } from './InfoButton.types'; | ||
| import { PopoverProps, PopoverTrigger } from '@fluentui/react-popover'; | ||
|
|
||
| /** | ||
| * Render the final JSX of InfoButton | ||
| */ | ||
| export const renderInfoButton_unstable = (state: InfoButtonState) => { | ||
| const { slots, slotProps } = getSlots<InfoButtonSlots>(state); | ||
|
|
||
| // TODO Add additional slots in the appropriate place | ||
| return <slots.root {...slotProps.root} />; | ||
| return ( | ||
| <slots.root {...(slotProps.root as PopoverProps)}> | ||
| <PopoverTrigger> | ||
| <slots.trigger {...slotProps.trigger} /> | ||
| </PopoverTrigger> | ||
| <slots.content {...slotProps.content} /> | ||
| </slots.root> | ||
| ); | ||
| }; |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| import * as React from 'react'; | ||
| import { Button } from '@fluentui/react-button'; | ||
| import { DefaultInfoButtonIcon } from './DefaultInfoButtonIcon'; | ||
| import { OnOpenChangeData, OpenPopoverEvents, Popover, PopoverSurface } from '@fluentui/react-popover'; | ||
| import { resolveShorthand } from '@fluentui/react-utilities'; | ||
| import { useControllableState } from '@fluentui/react-utilities'; | ||
| import type { InfoButtonProps, InfoButtonState } from './InfoButton.types'; | ||
|
|
||
| /** | ||
| * Create the state required to render InfoButton. | ||
| * | ||
| * The returned state can be modified with hooks such as useInfoButtonStyles_unstable, | ||
| * before being passed to renderInfoButton_unstable. | ||
| * | ||
| * @param props - props from this instance of InfoButton | ||
| */ | ||
| export const useInfoButton_unstable = (props: InfoButtonProps): InfoButtonState => { | ||
| const [popoverOpen, setPopoverOpen] = useControllableState({ | ||
| state: props.open, | ||
| defaultState: props.defaultOpen, | ||
| initialState: false, | ||
| }); | ||
|
|
||
| const handleOnPopoverChange = (e: OpenPopoverEvents, data: OnOpenChangeData) => { | ||
| props.onOpenChange?.(e, data); | ||
| setPopoverOpen(data.open); | ||
| }; | ||
|
|
||
| return { | ||
| popoverOpen, | ||
|
|
||
| components: { | ||
| root: Popover, | ||
| trigger: Button, | ||
| content: PopoverSurface, | ||
| }, | ||
|
|
||
| root: { | ||
| children: <></>, | ||
| size: 'small', | ||
| withArrow: true, | ||
| positioning: 'above-start', | ||
| ...props, | ||
| open: popoverOpen, | ||
| onOpenChange: handleOnPopoverChange, | ||
| }, | ||
| content: resolveShorthand(props.content, { | ||
| required: true, | ||
| defaultProps: { | ||
| role: 'dialog', | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Popover requires an aria-label, this will be added in the future. |
||
| }, | ||
| }), | ||
| trigger: resolveShorthand(props.trigger, { | ||
| required: true, | ||
| defaultProps: { | ||
| size: 'small', | ||
| appearance: 'transparent', | ||
| icon: <DefaultInfoButtonIcon />, | ||
| }, | ||
| }), | ||
| }; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,46 @@ | ||
| import { makeStyles, mergeClasses } from '@griffel/react'; | ||
| import { iconFilledClassName, iconRegularClassName } from '@fluentui/react-icons'; | ||
| import { tokens } from '@fluentui/react-theme'; | ||
| import type { InfoButtonSlots, InfoButtonState } from './InfoButton.types'; | ||
| import type { SlotClassNames } from '@fluentui/react-utilities'; | ||
|
|
||
| export const infoButtonClassNames: SlotClassNames<InfoButtonSlots> = { | ||
| // This classname is not applied, but it's left here to prevent a linting error. | ||
| root: 'fui-InfoButton', | ||
| // TODO: add class names for all slots on InfoButtonSlots. | ||
| // Should be of the form `<slotName>: 'fui-InfoButton__<slotName>` | ||
| content: 'fui-InfoButton__content', | ||
| trigger: 'fui-InfoButton__trigger', | ||
| }; | ||
|
|
||
| /** | ||
| * Styles for the root slot | ||
| * Styles for the trigger slot | ||
| */ | ||
| const useStyles = makeStyles({ | ||
| root: { | ||
| // TODO Add default styles for the root element | ||
| }, | ||
| const useTriggerStyles = makeStyles({ | ||
| selected: { | ||
| color: tokens.colorNeutralForeground2BrandPressed, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you also need to set backgroundColor?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really, but button's spec asks for it, so I think it should stay. |
||
|
|
||
| [`& .${iconFilledClassName}`]: { | ||
| display: 'inline', | ||
| }, | ||
|
|
||
| // TODO add additional classes for different states and/or slots | ||
| [`& .${iconRegularClassName}`]: { | ||
| display: 'none', | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| /** | ||
| * Apply styling to the InfoButton slots based on the state | ||
| */ | ||
| export const useInfoButtonStyles_unstable = (state: InfoButtonState): InfoButtonState => { | ||
| const styles = useStyles(); | ||
| state.root.className = mergeClasses(infoButtonClassNames.root, styles.root, state.root.className); | ||
| const { popoverOpen } = state; | ||
| const triggerStyles = useTriggerStyles(); | ||
|
|
||
| // TODO Add class names to slots, for example: | ||
| // state.mySlot.className = mergeClasses(styles.mySlot, state.mySlot.className); | ||
| state.content.className = mergeClasses(infoButtonClassNames.content, state.content.className); | ||
| state.trigger.className = mergeClasses( | ||
| infoButtonClassNames.trigger, | ||
| popoverOpen && triggerStyles.selected, | ||
| state.trigger.className, | ||
| ); | ||
|
|
||
| return state; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,10 @@ | ||
| ## Best practices | ||
|
|
||
| ### Do | ||
| <details> | ||
| <summary> | ||
| Best Practices | ||
| </summary> | ||
|
|
||
| ### Don't | ||
|
|
||
| - Because the Popover isn't always visible, don't include information that people must know in order to complete the field. | ||
|
|
||
| </details> |
Uh oh!
There was an error while loading. Please reload this page.