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.

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

Components: Stop re-exporting experimental utilities from @wordpress/components #62155

Draft
wants to merge 4 commits into
base: trunk
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
8 changes: 8 additions & 0 deletions packages/block-editor/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,10 @@ _Returns_

- `Object`: Typography block support derived CSS classes & styles.

### hasSplitBorders

Undocumented declaration.

### HeadingLevelDropdown

Dropdown for selecting a heading level (1 through 6) or paragraph (0).
Expand Down Expand Up @@ -629,6 +633,10 @@ _Related_

- <https://github.com/WordPress/gutenberg/blob/HEAD/packages/block-editor/src/components/inspector-controls/README.md>

### isDefinedBorder

Undocumented declaration.

### isValueSpacingPreset

Checks is given value is a spacing preset.
Expand Down
49 changes: 49 additions & 0 deletions packages/block-editor/src/components/border-box-utils/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* Internal dependencies
*/
import type {
Border,
AnyBorder,
Borders,
BorderProp,
BorderSide,
} from './types';

const sides: BorderSide[] = [ 'top', 'right', 'bottom', 'left' ];
const borderProps: BorderProp[] = [ 'color', 'style', 'width' ];

const isEmptyBorder = ( border?: Border ) => {
if ( ! border ) {
return true;
}
return ! borderProps.some( ( prop ) => border[ prop ] !== undefined );
};

export const isDefinedBorder = ( border: AnyBorder ) => {
// No border, no worries :)
if ( ! border ) {
return false;
}

// If we have individual borders per side within the border object we
// need to check whether any of those side borders have been set.
if ( hasSplitBorders( border ) ) {
const allSidesEmpty = sides.every( ( side ) =>
isEmptyBorder( ( border as Borders )[ side ] )
);

return ! allSidesEmpty;
}

// If we have a top-level border only, check if that is empty. e.g.
// { color: undefined, style: undefined, width: undefined }
// Border radius can still be set within the border object as it is
// handled separately.
return ! isEmptyBorder( border as Border );
};

export const hasSplitBorders = ( border: AnyBorder = {} ) => {
return Object.keys( border ).some(
( side ) => sides.indexOf( side as BorderSide ) !== -1
);
};
21 changes: 21 additions & 0 deletions packages/block-editor/src/components/border-box-utils/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* External dependencies
*/
import type { CSSProperties } from 'react';

export type Border = {
color?: CSSProperties[ 'borderColor' ];
style?: CSSProperties[ 'borderStyle' ];
width?: CSSProperties[ 'borderWidth' ];
};

export type Borders = {
top?: Border;
right?: Border;
bottom?: Border;
left?: Border;
};

export type AnyBorder = Border | Borders | undefined;
export type BorderProp = keyof Border;
export type BorderSide = keyof Borders;
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
*/
import {
__experimentalBorderBoxControl as BorderBoxControl,
__experimentalHasSplitBorders as hasSplitBorders,
__experimentalIsDefinedBorder as isDefinedBorder,
__experimentalToolsPanel as ToolsPanel,
__experimentalToolsPanelItem as ToolsPanelItem,
__experimentalItemGroup as ItemGroup,
Expand All @@ -22,6 +20,7 @@ import { getValueFromVariable, TOOLSPANEL_DROPDOWNMENU_PROPS } from './utils';
import { setImmutably } from '../../utils/object';
import { useBorderPanelLabel } from '../../hooks/border';
import { ShadowPopover, useShadowPresets } from './shadow-panel-components';
import { hasSplitBorders, isDefinedBorder } from '../border-box-utils';

export function useHasBorderPanel( settings ) {
const controls = Object.values( useHasBorderPanelControls( settings ) );
Expand Down
2 changes: 2 additions & 0 deletions packages/block-editor/src/components/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,5 @@ export { useBlockCommands } from './use-block-commands';
* The following rename hint component can be removed in 6.4.
*/
export { default as ReusableBlocksRenameHint } from './inserter/reusable-block-rename-hint';

export * from './border-box-utils';
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the packages/block-editor/src/utils be a better folder instead of the components folder for these utils?

2 changes: 1 addition & 1 deletion packages/block-editor/src/hooks/border.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import clsx from 'clsx';
* WordPress dependencies
*/
import { hasBlockSupport, getBlockSupport } from '@wordpress/blocks';
import { __experimentalHasSplitBorders as hasSplitBorders } from '@wordpress/components';
import { Platform, useCallback, useMemo } from '@wordpress/element';
import { addFilter } from '@wordpress/hooks';
import { useSelect } from '@wordpress/data';
Expand All @@ -30,6 +29,7 @@ import {
} from '../components/global-styles';
import { store as blockEditorStore } from '../store';
import { __ } from '@wordpress/i18n';
import { hasSplitBorders } from '../components/border-box-utils';

export const BORDER_SUPPORT_KEY = '__experimentalBorder';
export const SHADOW_SUPPORT_KEY = 'shadow';
Expand Down
5 changes: 4 additions & 1 deletion packages/block-editor/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,8 @@
// NOTE: This package is being progressively typed. You are encouraged to
// expand this array with files which can be type-checked. At some point in
// the future, this can be simplified to an `includes` of `src/**/*`.
"files": [ "src/components/block-context/index.js", "src/utils/dom.js" ]
"files": [ "src/components/block-context/index.js", "src/utils/dom.js" ],
"include": [
"src/components/border-box-utils/**/*"
]
}
2 changes: 1 addition & 1 deletion packages/block-library/src/table/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
BlockIcon,
AlignmentControl,
useBlockProps,
hasSplitBorders,
__experimentalUseColorProps as useColorProps,
__experimentalUseBorderProps as useBorderProps,
__experimentalGetElementClassName,
Expand All @@ -26,7 +27,6 @@ import {
TextControl,
ToggleControl,
ToolbarDropdownMenu,
__experimentalHasSplitBorders as hasSplitBorders,
} from '@wordpress/components';
import {
alignLeft,
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export {

export { default as __experimentalStyleProvider } from './style-provider';
export { default as BaseControl } from './base-control';
// @todo review this RN export
export { hasSplitBorders as __experimentalHasSplitBorders } from './border-box-control/utils';
Copy link
Member Author

@fullofcaffeine fullofcaffeine May 30, 2024

Choose a reason for hiding this comment

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

I don't know much about our RN setup. If we go with the approach in this PR, should we also remove this and copy it over closer to where the native is? I'm not sure if we should also remove it from RN at this point. I did search throughout GB, and it also looks like it's not being used by RN consumers in the codebase, but I'm not sure if it could be used by other third-party RN packages.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dcalhoun @WordPress/native-mobile can you please confirm that it's safe to remove this export? Thank you! 🙏

Copy link
Member

Choose a reason for hiding this comment

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

👋🏻 Thanks for the ping.

The mobile unit tests are passing on the CI server, so that is a good sign. 😄 I did not encountering any issues after checking out the proposed changes and "smoke testing" the editor on an iPhone simulator.

From quickly reviewing the related code, my perception is that __experimentalHasSplitBorders is not utilized by the native mobile editor. So, removing this export should likely be safe.

export { default as TextareaControl } from './textarea-control';
export { default as PanelBody } from './panel/body';
Expand Down
7 changes: 1 addition & 6 deletions packages/components/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,7 @@ export {
useAutocompleteProps as __unstableUseAutocompleteProps,
} from './autocomplete';
export { default as BaseControl, useBaseControlProps } from './base-control';
export {
BorderBoxControl as __experimentalBorderBoxControl,
hasSplitBorders as __experimentalHasSplitBorders,
isDefinedBorder as __experimentalIsDefinedBorder,
isEmptyBorder as __experimentalIsEmptyBorder,
} from './border-box-control';
export { BorderBoxControl as __experimentalBorderBoxControl } from './border-box-control';
export { BorderControl as __experimentalBorderControl } from './border-control';
export {
default as __experimentalBoxControl,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@
* WordPress dependencies
*/
import { getBlockType } from '@wordpress/blocks';
import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor';
import {
privateApis as blockEditorPrivateApis,
hasSplitBorders,
} from '@wordpress/block-editor';
import { useMemo } from '@wordpress/element';
import { useSelect } from '@wordpress/data';
import { store as coreStore } from '@wordpress/core-data';
import {
PanelBody,
__experimentalVStack as VStack,
__experimentalHasSplitBorders as hasSplitBorders,
} from '@wordpress/components';
import { __, sprintf } from '@wordpress/i18n';

Expand Down
Loading