Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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 @@ -8,6 +8,7 @@
- `BorderBoxControl` ([#79420](https://github.com/WordPress/gutenberg/pull/79420))
- `BorderControl` ([#79418](https://github.com/WordPress/gutenberg/pull/79418))
- `FontSizePicker` ([#79481](https://github.com/WordPress/gutenberg/pull/79481))
- `FormFileUpload` ([#79655](https://github.com/WordPress/gutenberg/pull/79655))
- `RangeControl` ([#79590](https://github.com/WordPress/gutenberg/pull/79590))
- `TreeSelect` ([#79550](https://github.com/WordPress/gutenberg/pull/79550))

Expand Down
10 changes: 0 additions & 10 deletions packages/components/src/form-file-upload/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { FormFileUpload } from '@wordpress/components';

const MyFormFileUpload = () => (
<FormFileUpload
__next40pxDefaultSize
accept="image/*"
onChange={ ( event ) => console.log( event.currentTarget.files ) }
>
Expand All @@ -22,14 +21,6 @@ const MyFormFileUpload = () => (

## Props

### `__next40pxDefaultSize`

- Type: `boolean`
- Required: No
- Default: `false`

Start opting into the larger default height that will become the default size in a future version.

### `accept`

- Type: `string`
Expand Down Expand Up @@ -85,7 +76,6 @@ an empty string in the `onClick` function.

```jsx
<FormFileUpload
__next40pxDefaultSize
onClick={ ( event ) => ( event.target.value = '' ) }
onChange={ onChange }
>
Expand Down
16 changes: 2 additions & 14 deletions packages/components/src/form-file-upload/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import { useRef } from '@wordpress/element';
import Button from '../button';
import type { WordPressComponentProps } from '../context';
import type { FormFileUploadProps } from './types';
import { maybeWarnDeprecated36pxSize } from '../utils/deprecated-36px-size';

/**
* FormFileUpload allows users to select files from their local device.
Expand All @@ -19,7 +18,6 @@ import { maybeWarnDeprecated36pxSize } from '../utils/deprecated-36px-size';
*
* const MyFormFileUpload = () => (
* <FormFileUpload
* __next40pxDefaultSize
* accept="image/*"
* onChange={ ( event ) => console.log( event.currentTarget.files ) }
* >
Expand All @@ -35,28 +33,18 @@ export function FormFileUpload( {
onChange,
onClick,
render,
__next40pxDefaultSize: _next40pxDefaultSize,
...props
}: WordPressComponentProps< FormFileUploadProps, 'button', false > ) {
const ref = useRef< HTMLInputElement >( null );
const openFileDialog = () => {
ref.current?.click();
};

if ( ! render ) {
maybeWarnDeprecated36pxSize( {
componentName: 'FormFileUpload',
__next40pxDefaultSize: props.__next40pxDefaultSize,
// @ts-expect-error - We don't "officially" support all Button props but this likely happens.
size: props.size,
} );
}

const ui = render ? (
render( { openFileDialog } )
) : (
// Disable reason: the parent component already takes care of the `__next40pxDefaultSize` prop.
// eslint-disable-next-line @wordpress/components-no-missing-40px-size-prop
<Button onClick={ openFileDialog } { ...props }>
<Button onClick={ openFileDialog } __next40pxDefaultSize { ...props }>
{ children }
</Button>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,12 @@ const meta: Meta< typeof FormFileUpload > = {
export default meta;

const Template: StoryFn< typeof FormFileUpload > = ( props ) => {
return <FormFileUpload __next40pxDefaultSize { ...props } />;
return <FormFileUpload { ...props } />;
};

export const Default = Template.bind( {} );
Default.args = {
children: 'Select file',
__next40pxDefaultSize: true,
};

export const RestrictFileTypes = Template.bind( {} );
Expand Down
17 changes: 1 addition & 16 deletions packages/components/src/form-file-upload/test/index.tsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,8 @@
/**
* External dependencies
*/
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

/**
* Internal dependencies
*/
import _FormFileUpload from '..';

/**
* Browser dependencies
*/
import FormFileUpload from '..';
const { File } = window;

const FormFileUpload = (
props: React.ComponentProps< typeof _FormFileUpload >
) => <_FormFileUpload __next40pxDefaultSize { ...props } />;

// @testing-library/user-event considers changing <input type="file"> to a string as a change, but it do not occur on real browsers, so the comparisons will be against this result
const fakePath = expect.objectContaining( {
target: expect.objectContaining( {
Expand Down
4 changes: 2 additions & 2 deletions packages/components/src/form-file-upload/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ export type FormFileUploadProps = {
/**
* Start opting into the larger default height that will become the default size in a future version.
*
* @default false
* @deprecated Default behavior since WordPress 7.1. Prop can be safely removed.
* @ignore
*/
__next40pxDefaultSize?: boolean;
/**
Expand Down Expand Up @@ -51,7 +52,6 @@ export type FormFileUploadProps = {
*
* ```jsx
* <FormFileUpload
* __next40pxDefaultSize
* onClick={ ( event ) => ( event.target.value = '' ) }
* onChange={ onChange }
* >
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ The following components are checked by this rule:
- ComboboxControl
- CustomSelectControl
- FontAppearanceControl
- FormFileUpload (special case - see below)
- FormTokenField
- InputControl
- NumberControl
Expand Down Expand Up @@ -43,18 +42,6 @@ import { Button, InputControl } from '@wordpress/components';
<Button size="compact">Click me</Button>
```

## FormFileUpload special case

`FormFileUpload` can use either the `__next40pxDefaultSize` prop or the `render` prop to be considered valid:

```jsx
import { FormFileUpload } from '@wordpress/components';

// Both are valid:
<FormFileUpload __next40pxDefaultSize />
<FormFileUpload render={({ open }) => <button onClick={open}>Upload</button>} />
```

## Options

### checkLocalImports
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,20 +71,6 @@ ruleTester.run( 'components-no-missing-40px-size-prop', rule, {
<WPButton __next40pxDefaultSize />
`,
},
// FormFileUpload with render prop (special case)
{
code: `
import { FormFileUpload } from '@wordpress/components';
<FormFileUpload render={({ open }) => <button onClick={open}>Upload</button>} />
`,
},
// FormFileUpload with __next40pxDefaultSize
{
code: `
import { FormFileUpload } from '@wordpress/components';
<FormFileUpload __next40pxDefaultSize />
`,
},
// Component with dynamic size prop (assumes it could be non-default)
{
code: `
Expand Down Expand Up @@ -202,18 +188,6 @@ ruleTester.run( 'components-no-missing-40px-size-prop', rule, {
},
],
},
// FormFileUpload without __next40pxDefaultSize or render
{
code: `
import { FormFileUpload } from '@wordpress/components';
<FormFileUpload onChange={handleChange} />
`,
errors: [
{
messageId: 'missingPropFormFileUpload',
},
],
},
// Multiple components, some invalid
{
code: `
Expand Down Expand Up @@ -325,14 +299,6 @@ ruleTester.run(
<InputControl />
`,
},
// FormFileUpload relative import with render prop
{
code: `
import { FormFileUpload } from '../form-file-upload';
<FormFileUpload render={({ open }) => <button onClick={open}>Upload</button>} />
`,
options: [ { checkLocalImports: true } ],
},
],
invalid: [],
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,6 @@ const COMPONENTS_REQUIRING_40PX = new Set( [
'UnitControl',
] );

/**
* Components that can use the `render` prop as an alternative to __next40pxDefaultSize.
*/
const COMPONENTS_WITH_RENDER_EXEMPTION = new Set( [ 'FormFileUpload' ] );

/**
* All tracked component names for path-based detection.
*/
const ALL_TRACKED_COMPONENTS = new Set( [
...COMPONENTS_REQUIRING_40PX,
...COMPONENTS_WITH_RENDER_EXEMPTION,
] );

module.exports = {
meta: {
type: 'problem',
Expand All @@ -59,8 +46,6 @@ module.exports = {
messages: {
missingProp:
'{{ component }} should have the `__next40pxDefaultSize` prop when using the default size.',
missingPropFormFileUpload:
'FormFileUpload should have the `__next40pxDefaultSize` prop to opt-in to the new default size.',
},
},
create( context ) {
Expand Down Expand Up @@ -113,7 +98,7 @@ module.exports = {
.join( '' );

// Check if it's one of our tracked components
if ( ALL_TRACKED_COMPONENTS.has( pascalCase ) ) {
if ( COMPONENTS_REQUIRING_40PX.has( pascalCase ) ) {
return pascalCase;
}

Expand Down Expand Up @@ -158,21 +143,6 @@ module.exports = {
return false;
}

/**
* Check if the `render` prop exists.
*
* @param {Array} attributes - JSX attributes array
* @return {boolean} Whether render prop exists
*/
function hasRenderProp( attributes ) {
return attributes.some(
( a ) =>
a.type === 'JSXAttribute' &&
a.name &&
a.name.name === 'render'
);
}

/**
* Check if the `variant` prop has the value "link".
* Button with variant="link" doesn't need __next40pxDefaultSize.
Expand Down Expand Up @@ -222,10 +192,7 @@ module.exports = {
const localName = specifier.local.name;

// Track components that require the prop
if (
COMPONENTS_REQUIRING_40PX.has( importedName ) ||
COMPONENTS_WITH_RENDER_EXEMPTION.has( importedName )
) {
if ( COMPONENTS_REQUIRING_40PX.has( importedName ) ) {
trackedImports.set( localName, importedName );
}
} );
Expand All @@ -246,7 +213,7 @@ module.exports = {
// Support patterns like `import ClipboardButton from '.';`
// (common in component folder examples/tests).
// If the local name matches a tracked component, treat it as such.
if ( ALL_TRACKED_COMPONENTS.has( localName ) ) {
if ( COMPONENTS_REQUIRING_40PX.has( localName ) ) {
trackedImports.set( localName, localName );
}
}
Expand Down Expand Up @@ -277,20 +244,6 @@ module.exports = {
return;
}

// Handle FormFileUpload special case
if ( COMPONENTS_WITH_RENDER_EXEMPTION.has( importedName ) ) {
// FormFileUpload is valid if it has a `render` prop
if ( hasRenderProp( attributes ) ) {
return;
}

context.report( {
node,
messageId: 'missingPropFormFileUpload',
} );
return;
}

// For other components, check if size prop has a non-default value
if ( hasNonDefaultSize( attributes ) ) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,6 @@ export function MediaUploadModal( {
accept={ acceptTypes }
multiple
onChange={ handleFileSelect }
__next40pxDefaultSize
render={ ( { openFileDialog } ) => (
<Button
onClick={ openFileDialog }
Expand Down
3 changes: 2 additions & 1 deletion tools/eslint/config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ const restrictedSyntax = [
'FocalPointPicker',
'FontFamilyControl',
'FontSizePicker',
'FormFileUpload',
'LetterSpacingControl',
'LineHeightControl',
'QueryControls',
Expand All @@ -191,7 +192,7 @@ const restrictedSyntax = [
'TextIndentControl',
'TreeSelect',
].map( ( componentName ) => ( {
selector: `JSXElement[openingElement.name.name="${ componentName }"] JSXAttribute[name.name="__next40pxDefaultSize"]`,
selector: `JSXOpeningElement[name.name="${ componentName }"] > JSXAttribute[name.name="__next40pxDefaultSize"]`,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The previous selector was too broad, in that it matched all descendants of the component, which could match unrelated stuff in render props and so on. Now we match direct descendant nodes only.

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.

Makes sense!

message: `The \`__next40pxDefaultSize\` prop is no longer needed on \`${ componentName }\`.`,
} ) ),
{
Expand Down
Loading