Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

- Components that compose Emotion style fragments with `cx()` should pass source-order-dependent fragments in a single `css()` call. Passing separate fragments can change override order after the following components stopped rendering styles through Emotion:
- `DropdownContentWrapper` ([#81522](https://github.com/WordPress/gutenberg/pull/81522))
- `ResizableBox` ([#81792](https://github.com/WordPress/gutenberg/pull/81792))

### Enhancements

Expand Down
23 changes: 16 additions & 7 deletions packages/components/src/resizable-box/resize-tooltip/index.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import clsx from 'clsx';
import type { Ref, ForwardedRef } from 'react';
import type { ElementType, Ref, ForwardedRef } from 'react';
import { forwardRef } from '@wordpress/element';
import { PolymorphicElement } from '../../utils/polymorphic-element';
import Label from './label';
import type { Axis, Position } from './utils';
import { useResizeLabel, POSITIONS } from './utils';
import { Root } from './styles/resize-tooltip.styles';
import styles from './style.module.scss';

type ResizeTooltipProps = React.ComponentProps< typeof Root > & {
type ResizeTooltipProps = React.ComponentPropsWithoutRef< 'div' > & {
as?: ElementType;
'aria-hidden'?: boolean;
axis?: Axis;
className?: string;
Expand All @@ -23,6 +25,7 @@ const noop = () => {};

function ResizeTooltip(
{
as = 'div',
axis,
className,
fadeTimeout = 180,
Expand All @@ -34,7 +37,7 @@ function ResizeTooltip(
zIndex = 1000,
...props
}: ResizeTooltipProps,
ref: ForwardedRef< HTMLDivElement >
ref: ForwardedRef< HTMLElement >
) {
const { label, resizeListener } = useResizeLabel( {
axis,
Expand All @@ -48,10 +51,16 @@ function ResizeTooltip(
return null;
}

const classes = clsx( 'components-resize-tooltip', className );
const classes = clsx( 'components-resize-tooltip', styles.root, className );

return (
<Root aria-hidden="true" className={ classes } ref={ ref } { ...props }>
<PolymorphicElement
aria-hidden="true"
as={ as }
{ ...props }
className={ classes }
ref={ ref }
>
{ resizeListener }
<Label
aria-hidden={ props[ 'aria-hidden' ] }
Expand All @@ -60,7 +69,7 @@ function ResizeTooltip(
ref={ labelRef }
zIndex={ zIndex }
/>
</Root>
</PolymorphicElement>
);
}

Expand Down
30 changes: 18 additions & 12 deletions packages/components/src/resizable-box/resize-tooltip/label.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import clsx from 'clsx';
import type { ForwardedRef } from 'react';
import { forwardRef } from '@wordpress/element';
import { isRTL } from '@wordpress/i18n';
import { Text } from '../../text';
import type { Position } from './utils';
import { POSITIONS } from './utils';
import {
TooltipWrapper,
Tooltip,
LabelText,
} from './styles/resize-tooltip.styles';
import styles from './style.module.scss';

const CORNER_OFFSET = 4;
const CURSOR_OFFSET_TOP = CORNER_OFFSET * 2.5;
Expand Down Expand Up @@ -66,20 +64,28 @@ function Label(
}

return (
<TooltipWrapper
<div
aria-hidden="true"
className="components-resizable-tooltip__tooltip-wrapper"
className={ clsx(
'components-resizable-tooltip__tooltip-wrapper',
styles[ 'tooltip-wrapper' ]
) }
ref={ ref }
style={ style }
{ ...props }
>
<Tooltip
className="components-resizable-tooltip__tooltip"
<div
className={ clsx(
'components-resizable-tooltip__tooltip',
styles.tooltip
) }
style={ labelStyle }
>
<LabelText as="span">{ label }</LabelText>
</Tooltip>
</TooltipWrapper>
<Text as="span" className={ styles[ 'label-text' ] }>
{ label }
</Text>
</div>
</div>
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
@use "@wordpress/base-styles/variables" as *;
@use "../../utils/theme-variables" as *;

.root {
box-sizing: border-box;
pointer-events: none;
position: absolute;
inset: 0;
}

.tooltip-wrapper {
align-items: center;
box-sizing: border-box;
display: inline-flex;
justify-content: center;
opacity: 0;
pointer-events: none;
transition: opacity 120ms linear;
}

.tooltip {
background: $components-color-foreground;
border-radius: $radius-small;
box-sizing: border-box;
font-family: $default-font;
font-size: 12px;
color: $components-color-foreground-inverted;
padding: $grid-unit-05 $grid-unit-10;
position: relative;
}

// Match the previous Emotion `&&&` specificity so label styles continue to
// override Text / consumer typography.
.label-text.label-text.label-text {
color: $components-color-foreground-inverted;
display: block;
font-size: 13px;
line-height: 1.4;
white-space: nowrap;
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { render, screen } from '@testing-library/react';
import { createElement, createRef } from '@wordpress/element';
import ResizeTooltip from '../index';
import styles from '../style.module.scss';

describe( 'ResizeTooltip', () => {
it( 'renders the legacy root class together with className and style', () => {
render(
<ResizeTooltip
className="custom-tooltip"
data-testid="resize-tooltip"
style={ { zIndex: 7 } }
/>
);

const root = screen.getByTestId( 'resize-tooltip' );

expect( root ).toHaveClass( 'components-resize-tooltip' );
expect( root ).toHaveClass( styles.root );
expect( root ).toHaveClass( 'custom-tooltip' );
expect( root ).toHaveStyle( { zIndex: '7' } );
} );

it( 'renders as a different element and forwards the root ref', () => {
const ref = createRef< HTMLElement >();

render(
<ResizeTooltip
as="section"
data-testid="resize-tooltip"
ref={ ref }
/>
);

const root = screen.getByTestId( 'resize-tooltip' );

expect( root.tagName ).toBe( 'SECTION' );
expect( ref.current ).toBe( root );
} );

it( 'forwards target-specific props and filters invalid intrinsic props', () => {
render(
createElement( ResizeTooltip, {
as: 'label',
'data-testid': 'resize-tooltip',
htmlFor: 'size-field',
labelPosition: 'top',
} as React.ComponentProps< typeof ResizeTooltip > & {
labelPosition: string;
} )
);

const root = screen.getByTestId( 'resize-tooltip' );

expect( root ).toHaveAttribute( 'for', 'size-field' );
expect( root ).not.toHaveAttribute( 'labelPosition' );
expect( root ).not.toHaveAttribute( 'labelposition' );
} );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Target-prop forwarding is useful, but invalid-prop filtering already belongs to PolymorphicElement’s tests. We can remove this test and combine the useful part (ie target props forwarding) with the previous as test.

} );
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import { render, screen } from '@testing-library/react';
import Label from '../label';
import { POSITIONS } from '../utils';
import styles from '../style.module.scss';

describe( 'ResizeTooltip label', () => {
it( 'keeps the Text contract when an ancestor uses a bold font weight', () => {
render(
<div style={ { fontWeight: 700 } }>
<Label
label="120 x 80"
position={ POSITIONS.bottom }
zIndex={ 1 }
/>
</div>
);

const label = screen.getByText( '120 x 80' );

expect( label.tagName ).toBe( 'SPAN' );
expect( label ).toHaveClass( 'components-text' );
expect( label ).toHaveClass( 'components-truncate' );
expect( label ).toHaveClass( styles[ 'label-text' ] );
expect( label ).toHaveAttribute( 'data-wp-component', 'Text' );
expect( label ).toHaveStyle( { fontWeight: 'normal' } );
} );

it( 'positions the label at the bottom', () => {
const { container } = render(
<Label label="200 px" position={ POSITIONS.bottom } zIndex={ 3 } />
);

// Disable reason: the wrapper is presentational and has no accessible name.
// eslint-disable-next-line testing-library/no-node-access, testing-library/no-container
const wrapper = container.querySelector(
'.components-resizable-tooltip__tooltip-wrapper'
);

expect( wrapper ).toHaveStyle( {
bottom: '-10px',
left: '50%',
position: 'absolute',
zIndex: '3',
} );
} );

it( 'positions the label in the corner', () => {
const { container } = render(
<Label
label="200 x 100"
position={ POSITIONS.corner }
zIndex={ 3 }
/>
);

// Disable reason: the wrapper is presentational and has no accessible name.
// eslint-disable-next-line testing-library/no-node-access, testing-library/no-container
const wrapper = container.querySelector(
'.components-resizable-tooltip__tooltip-wrapper'
);

expect( wrapper ).toHaveStyle( {
top: '4px',
right: '4px',
position: 'absolute',
} );
} );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These tests currently bring little value:

  • outer styles (such as transform) could still modify the positioning
  • it doesn't cover RTL (especially for the corner test)

We can probably remove them. These styles would be better suited for a proper visual regression testing (if/when we have one)


it( 'keeps label-text declarations when a consumer selector targets the same class', () => {
const hash = styles[ 'label-text' ];
const style = document.createElement( 'style' );
style.textContent = `
.${ hash } { color: rgb(255, 0, 0); font-size: 99px; }
.${ hash }.${ hash }.${ hash } { color: rgb(1, 1, 1); font-size: 13px; }
`;
document.head.append( style );

render(
<Label label="80 px" position={ POSITIONS.bottom } zIndex={ 1 } />
);

const label = screen.getByText( '80 px' );
const computed = window.getComputedStyle( label );

expect( label ).toHaveClass( hash );
expect( computed.color ).toBe( 'rgb(1, 1, 1)' );
expect( computed.fontSize ).toBe( '13px' );

style.remove();
} );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This test has no real value against future regressions: Jest mocks the SCSS import, and the test injects the desired triple-class selector itself. It passes even if the production selector is removed.

We should remove this test

} );
11 changes: 11 additions & 0 deletions packages/components/src/resizable-box/stories/index.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const meta: Meta< typeof ResizableBox > = {
children: { control: false },
enable: { control: 'object' },
onResizeStop: { action: 'onResizeStop' },
__experimentalShowTooltip: { control: 'boolean' },
},
parameters: {
controls: { expanded: true },
Expand Down Expand Up @@ -85,3 +86,13 @@ DisabledDirections.args = {
topLeft: false,
},
};

/**
* The resize size label is hidden by default. Enable it with
* `__experimentalShowTooltip`, or toggle that control in Storybook.
*/
export const WithResizeTooltip = Template.bind( {} );
WithResizeTooltip.args = {
...Default.args,
__experimentalShowTooltip: true,
};
5 changes: 0 additions & 5 deletions tools/eslint/suppressions.json
Original file line number Diff line number Diff line change
Expand Up @@ -809,11 +809,6 @@
"count": 2
}
},
"packages/components/src/resizable-box/resize-tooltip/styles/resize-tooltip.styles.ts": {
"no-restricted-imports": {
"count": 1
}
},
"packages/components/src/scrollable/styles.ts": {
"no-restricted-imports": {
"count": 1
Expand Down
Loading