Skip to content

Commit

Permalink
add "open" popover styling to buttons (#2076)
Browse files Browse the repository at this point in the history
  • Loading branch information
mayank99 authored Jun 12, 2024
1 parent e131ed7 commit 3595174
Show file tree
Hide file tree
Showing 30 changed files with 103 additions and 26 deletions.
5 changes: 5 additions & 0 deletions .changeset/nine-shrimps-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@itwin/itwinui-react': minor
---

Added open popover styling to `Button` (and `IconButton`). When an associated `Popover` or `DropdownMenu` is open, the button will now get a subtle visual indication.
9 changes: 9 additions & 0 deletions .changeset/real-rockets-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@itwin/itwinui-css': minor
---

Added a new `data-iui-has-popover` attribute to `iui-button` to be applied when the button has an associated popover/dropdown-menu that is currently open.

```html
<button class="iui-button" data-iui-has-popover="open">
```
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions packages/itwinui-css/src/button/button.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
.iui-button {
@include base.iui-button;

// Reusing hover styling when an associated popover/dropdown is open.
&:is([data-iui-has-popover='open'], :has(+ :popover-open)) {
--_iui-field-state: var(--_iui-field-state--hover);
}

&[data-iui-size='small'] {
@include base.iui-button-size(small);
}
Expand Down
28 changes: 27 additions & 1 deletion packages/itwinui-react/src/core/Buttons/Button.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
* Copyright (c) Bentley Systems, Incorporated. All rights reserved.
* See LICENSE.md in the project root for license terms and full copyright notice.
*--------------------------------------------------------------------------------------------*/
import { render } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import * as React from 'react';
import { SvgClose as SvgPlaceholder } from '../../utils/index.js';
import { Button } from './Button.js';
import { Popover } from '../Popover/Popover.js';
import { userEvent } from '@testing-library/user-event';

it('renders default button correctly', () => {
const onClickMock = vi.fn();
Expand Down Expand Up @@ -249,3 +251,27 @@ it.each(['default', 'small', 'large'] as const)(
);
},
);

it('should read popover open state', async () => {
render(
<Popover content='Popped over'>
<Button>Click me</Button>
</Popover>,
);

const button = screen.getByRole('button', { name: 'Click me' });
expect(button).not.toHaveAttribute('data-iui-has-popover', 'open');
expect(screen.queryByText('Popped over')).toBeNull();

// Open popover
await userEvent.click(button);
expect(button).toHaveAttribute('data-iui-has-popover', 'open');

const popover = screen.getByRole('dialog');
expect(popover).toBeVisible();
expect(popover).toHaveTextContent('Popped over');

// Close popover
await userEvent.click(button);
expect(button).not.toHaveAttribute('data-iui-has-popover', 'open');
});
4 changes: 4 additions & 0 deletions packages/itwinui-react/src/core/Buttons/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as React from 'react';
import { Box, ButtonBase } from '../../utils/index.js';
import type { PolymorphicForwardRefComponent } from '../../utils/index.js';
import { ProgressRadial } from '../ProgressIndicators/ProgressRadial.js';
import { PopoverOpenContext } from '../Popover/Popover.js';

export type ButtonProps = {
/**
Expand Down Expand Up @@ -78,13 +79,16 @@ export const Button = React.forwardRef((props, ref) => {
...rest
} = props;

const hasPopoverOpen = React.useContext(PopoverOpenContext);

return (
<ButtonBase
ref={ref}
className={cx('iui-button', 'iui-field', className)}
data-iui-variant={styleType !== 'default' ? styleType : undefined}
data-iui-size={size}
data-iui-loading={loading ? 'true' : undefined}
data-iui-has-popover={hasPopoverOpen ? 'open' : undefined}
disabled={disabledProp || loading}
{...rest}
style={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ it('should render in its most basic state', () => {
expect(container.querySelector('.iui-button-icon')).toBeTruthy();
});

it('should update icon when menu opens or closes', async () => {
it('should update when menu opens or closes', async () => {
const { container } = renderComponent();

const button = container.querySelector('.iui-button') as HTMLButtonElement;
expect(button).toBeTruthy();
expect(button).not.toHaveAttribute('data-iui-has-popover', 'open');

const {
container: { firstChild: downArrow },
Expand All @@ -57,7 +58,10 @@ it('should update icon when menu opens or closes', async () => {
expect(buttonIcon).toHaveAttribute('aria-hidden', 'true');
expect(svg).toEqual(downArrow);

// Open menu
await userEvent.click(button);
expect(button).toHaveAttribute('data-iui-has-popover', 'open');

const {
container: { firstChild: upArrow },
} = render(<SvgCaretUpSmall aria-hidden />);
Expand All @@ -68,7 +72,9 @@ it('should update icon when menu opens or closes', async () => {
expect(buttonIcon).toHaveAttribute('aria-hidden', 'true');
expect(svg).toEqual(upArrow);

// Close menu
await userEvent.click(button);
expect(button).not.toHaveAttribute('data-iui-has-popover', 'open');

buttonIcon = container.querySelector('.iui-button-icon');
svg = buttonIcon?.querySelector('svg');
Expand Down
3 changes: 3 additions & 0 deletions packages/itwinui-react/src/core/Buttons/IconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type { ButtonProps } from './Button.js';
import type { PolymorphicForwardRefComponent } from '../../utils/index.js';
import { VisuallyHidden } from '../VisuallyHidden/VisuallyHidden.js';
import { ButtonGroupContext } from '../ButtonGroup/ButtonGroup.js';
import { PopoverOpenContext } from '../Popover/Popover.js';

export type IconButtonProps = {
/**
Expand Down Expand Up @@ -64,6 +65,7 @@ export const IconButton = React.forwardRef((props, ref) => {
} = props;

const buttonGroupOrientation = React.useContext(ButtonGroupContext);
const hasPopoverOpen = React.useContext(PopoverOpenContext);

const button = (
<ButtonBase
Expand All @@ -72,6 +74,7 @@ export const IconButton = React.forwardRef((props, ref) => {
data-iui-variant={styleType !== 'default' ? styleType : undefined}
data-iui-size={size}
data-iui-active={isActive}
data-iui-has-popover={hasPopoverOpen ? 'open' : undefined}
aria-pressed={isActive}
{...rest}
>
Expand Down
8 changes: 7 additions & 1 deletion packages/itwinui-react/src/core/Buttons/SplitButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ it('should fire onClick callback', async () => {
expect(onClickMock).toHaveBeenCalledTimes(1);
});

it('should update icon when menu opens or closes', async () => {
it('should update when menu opens or closes', async () => {
const { container } = renderComponent();
expect(container.querySelector('.iui-button-split')).toBeTruthy();
expect(container.querySelector('.iui-button')).toBeTruthy();
Expand All @@ -66,6 +66,7 @@ it('should update icon when menu opens or closes', async () => {
'.iui-button',
)[1] as HTMLButtonElement;
expect(dropdownButton).toBeTruthy();
expect(dropdownButton).not.toHaveAttribute('data-iui-has-popover', 'open');

const {
container: { firstChild: downArrow },
Expand All @@ -77,7 +78,10 @@ it('should update icon when menu opens or closes', async () => {
expect(buttonIcon).toHaveAttribute('aria-hidden', 'true');
expect(svg).toEqual(downArrow);

// Open menu
await userEvent.click(dropdownButton);
expect(dropdownButton).toHaveAttribute('data-iui-has-popover', 'open');

const {
container: { firstChild: upArrow },
} = render(<SvgCaretUpSmall />);
Expand All @@ -87,7 +91,9 @@ it('should update icon when menu opens or closes', async () => {

expect(svg).toEqual(upArrow);

// Close menu
await userEvent.click(dropdownButton);
expect(dropdownButton).not.toHaveAttribute('data-iui-has-popover', 'open');
expect(container.querySelector('.iui-button-icon svg')).toEqual(downArrow);
});

Expand Down
26 changes: 14 additions & 12 deletions packages/itwinui-react/src/core/Buttons/SplitButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import type {
} from '../../utils/index.js';
import type { Placement } from '@floating-ui/react';
import { Menu } from '../Menu/Menu.js';
import { usePopover } from '../Popover/Popover.js';
import { PopoverOpenContext, usePopover } from '../Popover/Popover.js';

export type SplitButtonProps = ButtonProps & {
/**
Expand Down Expand Up @@ -128,17 +128,19 @@ export const SplitButton = React.forwardRef((props, forwardedRef) => {
>
{children}
</Button>
<IconButton
styleType={styleType}
size={size}
disabled={props.disabled}
aria-labelledby={props.labelProps?.id || labelId}
aria-expanded={popover.open}
ref={popover.refs.setReference}
{...popover.getReferenceProps(menuButtonProps)}
>
{visible ? <SvgCaretUpSmall /> : <SvgCaretDownSmall />}
</IconButton>
<PopoverOpenContext.Provider value={popover.open}>
<IconButton
styleType={styleType}
size={size}
disabled={props.disabled}
aria-labelledby={props.labelProps?.id || labelId}
aria-expanded={popover.open}
ref={popover.refs.setReference}
{...popover.getReferenceProps(menuButtonProps)}
>
{visible ? <SvgCaretUpSmall /> : <SvgCaretDownSmall />}
</IconButton>
</PopoverOpenContext.Provider>
{popover.open && (
<Portal portal={portal}>
<Menu
Expand Down
14 changes: 8 additions & 6 deletions packages/itwinui-react/src/core/DropdownMenu/DropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type {
PortalProps,
} from '../../utils/index.js';
import { Menu } from '../Menu/Menu.js';
import { usePopover } from '../Popover/Popover.js';
import { PopoverOpenContext, usePopover } from '../Popover/Popover.js';
import {
FloatingNode,
FloatingTree,
Expand Down Expand Up @@ -138,11 +138,13 @@ const DropdownMenuContent = React.forwardRef((props, forwardedRef) => {

return (
<>
{cloneElementWithRef(children, (children) => ({
...popover.getReferenceProps(children.props),
'aria-expanded': popover.open,
ref: mergeRefs(triggerRef, popover.refs.setReference),
}))}
<PopoverOpenContext.Provider value={popover.open}>
{cloneElementWithRef(children, (children) => ({
...popover.getReferenceProps(children.props),
'aria-expanded': popover.open,
ref: mergeRefs(triggerRef, popover.refs.setReference),
}))}
</PopoverOpenContext.Provider>
<FloatingNode id={nodeId}>
{popover.open && (
<Portal portal={portal}>
Expand Down
19 changes: 14 additions & 5 deletions packages/itwinui-react/src/core/Popover/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,13 @@ type PopoverInternalProps = {

// ----------------------------------------------------------------------------

/** Stores the current open/closed state of the popover. */
export const PopoverOpenContext = React.createContext<boolean | undefined>(
undefined,
);

// ----------------------------------------------------------------------------

export const usePopover = (options: PopoverOptions & PopoverInternalProps) => {
const {
placement = 'bottom-start',
Expand Down Expand Up @@ -369,11 +376,13 @@ export const Popover = React.forwardRef((props, forwardedRef) => {

return (
<>
{cloneElementWithRef(children, (children) => ({
id: children.props.id || triggerId,
...popover.getReferenceProps(children.props),
ref: popover.refs.setReference,
}))}
<PopoverOpenContext.Provider value={popover.open}>
{cloneElementWithRef(children, (children) => ({
id: children.props.id || triggerId,
...popover.getReferenceProps(children.props),
ref: popover.refs.setReference,
}))}
</PopoverOpenContext.Provider>

{popover.open ? (
<PopoverPortal portal={portal}>
Expand Down

0 comments on commit 3595174

Please sign in to comment.