From 537b3ed7e926a8066b7fc188ce7a81c38bc12e37 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 12:40:37 -0700 Subject: [PATCH 1/7] [EuiFieldNumber] Fix `icon` type - should also accept an obj shape --- src/components/form/field_number/field_number.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/form/field_number/field_number.tsx b/src/components/form/field_number/field_number.tsx index f7a6f9e003f7..fa2c72602e65 100644 --- a/src/components/form/field_number/field_number.tsx +++ b/src/components/form/field_number/field_number.tsx @@ -19,7 +19,6 @@ import classNames from 'classnames'; import { useCombinedRefs } from '../../../services'; import { CommonProps } from '../../common'; -import { IconType } from '../../icon'; import { EuiValidatableControl } from '../validatable_control'; import { @@ -34,7 +33,7 @@ export type EuiFieldNumberProps = Omit< 'min' | 'max' | 'readOnly' | 'step' > & CommonProps & { - icon?: IconType; + icon?: EuiFormControlLayoutProps['icon']; isInvalid?: boolean; /** * Expand to fill 100% of the parent. From 7195758497894982dff8d3edf1fc79b1b5a29925 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 12:44:44 -0700 Subject: [PATCH 2/7] Add new util for icons configured to the right side --- .../form_control_layout/_num_icons.test.ts | 29 ++++++++++++++++++- .../form/form_control_layout/_num_icons.ts | 17 +++++++++-- .../form_control_layout_icons.tsx | 6 ++-- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/components/form/form_control_layout/_num_icons.test.ts b/src/components/form/form_control_layout/_num_icons.test.ts index ae56dd296291..4b3f26693e75 100644 --- a/src/components/form/form_control_layout/_num_icons.test.ts +++ b/src/components/form/form_control_layout/_num_icons.test.ts @@ -6,7 +6,10 @@ * Side Public License, v 1. */ -import { getFormControlClassNameForIconCount } from './_num_icons'; +import { + getFormControlClassNameForIconCount, + isRightSideIcon, +} from './_num_icons'; describe('getFormControlClassNameForIconCount', () => { it('should return undefined if object is empty', () => { @@ -47,3 +50,27 @@ describe('getFormControlClassNameForIconCount', () => { expect(numberClass).toEqual('euiFormControlLayout--5icons'); }); }); + +describe('isRightSideIcon', () => { + it('returns true if side has been set to right', () => { + expect(isRightSideIcon({ side: 'right', type: 'warning' })).toEqual(true); + }); + + it('returns false if side has been set to left', () => { + expect(isRightSideIcon({ side: 'left', type: 'warning' })).toEqual(false); + }); + + it('returns false if icon is missing a side definition (defaults to left)', () => { + expect(isRightSideIcon({ type: 'warning', color: 'warning' })).toEqual( + false + ); + }); + + it('returns false if icon is undefined', () => { + expect(isRightSideIcon()).toEqual(false); + }); + + it('returns false if icon is not in an object shape', () => { + expect(isRightSideIcon('warning')).toEqual(false); + }); +}); diff --git a/src/components/form/form_control_layout/_num_icons.ts b/src/components/form/form_control_layout/_num_icons.ts index e9a15884e08a..95c792c4cf20 100644 --- a/src/components/form/form_control_layout/_num_icons.ts +++ b/src/components/form/form_control_layout/_num_icons.ts @@ -6,6 +6,11 @@ * Side Public License, v 1. */ +import { + isIconShape, + type EuiFormControlLayoutIconsProps, +} from './form_control_layout_icons'; + /** * The `getFormControlClassNameForIconCount` function helps setup the className appendum * depending on the form control's current settings/state. @@ -26,17 +31,23 @@ export type _EuiFormControlLayoutNumIcons = { isDropdown?: boolean; }; -export function getFormControlClassNameForIconCount({ +export const getFormControlClassNameForIconCount = ({ icon, clear, isLoading, isInvalid, isDropdown, -}: _EuiFormControlLayoutNumIcons): string | undefined { +}: _EuiFormControlLayoutNumIcons): string | undefined => { const numIcons = [icon, clear, isInvalid, isLoading, isDropdown].filter( (item) => item === true ).length; // This className is also specifically used in `src/global_styling/mixins/_form.scss` return numIcons > 0 ? `euiFormControlLayout--${numIcons}icons` : undefined; -} +}; + +export const isRightSideIcon = ( + icon?: EuiFormControlLayoutIconsProps['icon'] +): boolean => { + return !!icon && isIconShape(icon) && icon.side === 'right'; +}; diff --git a/src/components/form/form_control_layout/form_control_layout_icons.tsx b/src/components/form/form_control_layout/form_control_layout_icons.tsx index e8d23bd2fc33..5cfd7ce1ac3a 100644 --- a/src/components/form/form_control_layout/form_control_layout_icons.tsx +++ b/src/components/form/form_control_layout/form_control_layout_icons.tsx @@ -33,11 +33,11 @@ export type IconShape = DistributiveOmit< ref?: EuiFormControlLayoutCustomIconProps['iconRef']; }; -function isIconShape( +export const isIconShape = ( icon: EuiFormControlLayoutIconsProps['icon'] -): icon is IconShape { +): icon is IconShape => { return !!icon && icon.hasOwnProperty('type'); -} +}; export interface EuiFormControlLayoutIconsProps { icon?: IconType | IconShape; From 47ac6253ca725d19152c71037fefa5c6caeb132b Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 12:45:38 -0700 Subject: [PATCH 3/7] Fix EuiFieldNumber and EuiFieldText to correctly set paddings for right side icons --- src/components/form/field_number/field_number.tsx | 9 +++++++-- src/components/form/field_text/field_text.tsx | 10 ++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/components/form/field_number/field_number.tsx b/src/components/form/field_number/field_number.tsx index fa2c72602e65..b535aca00301 100644 --- a/src/components/form/field_number/field_number.tsx +++ b/src/components/form/field_number/field_number.tsx @@ -25,7 +25,10 @@ import { EuiFormControlLayout, EuiFormControlLayoutProps, } from '../form_control_layout'; -import { getFormControlClassNameForIconCount } from '../form_control_layout/_num_icons'; +import { + getFormControlClassNameForIconCount, + isRightSideIcon, +} from '../form_control_layout/_num_icons'; import { useFormContext } from '../eui_form_context'; export type EuiFieldNumberProps = Omit< @@ -133,15 +136,17 @@ export const EuiFieldNumber: FunctionComponent = ( } }, [value, min, max, step, checkNativeValidity]); + const hasRightSideIcon = isRightSideIcon(icon); const numIconsClass = controlOnly ? false : getFormControlClassNameForIconCount({ isInvalid: isInvalid || isNativelyInvalid, isLoading, + icon: hasRightSideIcon, }); const classes = classNames('euiFieldNumber', className, numIconsClass, { - 'euiFieldNumber--withIcon': icon, + 'euiFieldNumber--withIcon': icon && !hasRightSideIcon, 'euiFieldNumber--fullWidth': fullWidth, 'euiFieldNumber--compressed': compressed, 'euiFieldNumber--inGroup': prepend || append, diff --git a/src/components/form/field_text/field_text.tsx b/src/components/form/field_text/field_text.tsx index 3a40234d5edc..01a83f55d718 100644 --- a/src/components/form/field_text/field_text.tsx +++ b/src/components/form/field_text/field_text.tsx @@ -16,7 +16,10 @@ import { } from '../form_control_layout'; import { EuiValidatableControl } from '../validatable_control'; -import { getFormControlClassNameForIconCount } from '../form_control_layout/_num_icons'; +import { + isRightSideIcon, + getFormControlClassNameForIconCount, +} from '../form_control_layout/_num_icons'; import { useFormContext } from '../eui_form_context'; export type EuiFieldTextProps = InputHTMLAttributes & @@ -78,15 +81,18 @@ export const EuiFieldText: FunctionComponent = (props) => { ...rest } = props; + const hasRightSideIcon = isRightSideIcon(icon); + const numIconsClass = controlOnly ? false : getFormControlClassNameForIconCount({ isInvalid, isLoading, + icon: hasRightSideIcon, }); const classes = classNames('euiFieldText', className, numIconsClass, { - 'euiFieldText--withIcon': icon, + 'euiFieldText--withIcon': icon && !hasRightSideIcon, 'euiFieldText--fullWidth': fullWidth, 'euiFieldText--compressed': compressed, 'euiFieldText--inGroup': prepend || append, From 131b093cd0698806b0d1ddff33ad95ed5a7475ad Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 13:16:17 -0700 Subject: [PATCH 4/7] Add storybooks for testing the `icon` shape/prop --- .../field_number/field_number.stories.tsx | 62 ++++++++++++++++++ .../form/field_text/field_text.stories.tsx | 64 +++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 src/components/form/field_text/field_text.stories.tsx diff --git a/src/components/form/field_number/field_number.stories.tsx b/src/components/form/field_number/field_number.stories.tsx index b283f0d8e5f6..0aa0ab52b7ea 100644 --- a/src/components/form/field_number/field_number.stories.tsx +++ b/src/components/form/field_number/field_number.stories.tsx @@ -7,6 +7,11 @@ */ import type { Meta, StoryObj } from '@storybook/react'; +import { + disableStorybookControls, + hideStorybookControls, + moveStorybookControlsToCategory, +} from '../../../../.storybook/utils'; import { EuiFieldNumber, EuiFieldNumberProps } from './field_number'; @@ -15,11 +20,28 @@ const meta: Meta = { component: EuiFieldNumber, argTypes: { step: { control: 'number' }, + // For quicker/easier QA + icon: { control: 'text' }, + prepend: { control: 'text' }, + append: { control: 'text' }, + }, + args: { + // Component defaults + compressed: false, + fullWidth: false, + isInvalid: false, + isLoading: false, + disabled: false, + readOnly: false, + controlOnly: false, + // Added for easier testing + placeholder: '0', }, }; export default meta; type Story = StoryObj; +disableStorybookControls(meta, ['inputRef']); export const Playground: Story = {}; @@ -32,3 +54,43 @@ export const ControlledComponent: Story = { onChange: () => {}, }, }; +// Hide props that don't impact this story +hideStorybookControls(ControlledComponent, [ + 'controlOnly', + 'inputRef', + 'compressed', + 'fullWidth', + 'icon', + 'isInvalid', + 'isLoading', + 'disabled', + 'readOnly', + 'placeholder', + 'prepend', + 'append', +]); + +export const IconShape: Story = { + argTypes: { icon: { control: 'object' } }, + args: { icon: { type: 'warning', color: 'warning', side: 'left' } }, +}; +// Move other props below the icon prop +moveStorybookControlsToCategory(IconShape, [ + 'compressed', + 'fullWidth', + 'isInvalid', + 'isLoading', + 'disabled', + 'readOnly', + 'placeholder', + 'prepend', + 'append', +]); +// Hide props that remove or won't affect the icon or its positioning +hideStorybookControls(IconShape, [ + 'controlOnly', + 'inputRef', + 'min', + 'max', + 'step', +]); diff --git a/src/components/form/field_text/field_text.stories.tsx b/src/components/form/field_text/field_text.stories.tsx new file mode 100644 index 000000000000..4484f6918148 --- /dev/null +++ b/src/components/form/field_text/field_text.stories.tsx @@ -0,0 +1,64 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import type { Meta, StoryObj } from '@storybook/react'; +import { + disableStorybookControls, + hideStorybookControls, + moveStorybookControlsToCategory, +} from '../../../../.storybook/utils'; + +import { EuiFieldText, EuiFieldTextProps } from './field_text'; + +const meta: Meta = { + title: 'Forms/EuiFieldText', + component: EuiFieldText, + argTypes: { + // For quicker/easier QA + icon: { control: 'text' }, + prepend: { control: 'text' }, + append: { control: 'text' }, + }, + args: { + // Component defaults + compressed: false, + fullWidth: false, + isInvalid: false, + isLoading: false, + disabled: false, + readOnly: false, + controlOnly: false, + // Added for easier testing + placeholder: 'EuiFieldText', + }, +}; + +export default meta; +type Story = StoryObj; +disableStorybookControls(meta, ['inputRef']); + +export const Playground: Story = {}; + +export const IconShape: Story = { + argTypes: { icon: { control: 'object' } }, + args: { icon: { type: 'warning', color: 'warning', side: 'left' } }, +}; +// Move other props below the icon prop +moveStorybookControlsToCategory(IconShape, [ + 'compressed', + 'fullWidth', + 'isInvalid', + 'isLoading', + 'disabled', + 'readOnly', + 'placeholder', + 'prepend', + 'append', +]); +// Hide props that remove or won't affect the icon or its positioning +hideStorybookControls(IconShape, ['controlOnly', 'inputRef']); From e6f5c72bc8ae414e712dc4de1d976ac7408d176d Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 13:17:42 -0700 Subject: [PATCH 5/7] Fix `controlOnly` prop to actually fully ignore `icon`/`prepend`/`append` - which is what it says on the props docs - rendering was visually broken if `icon` and `prepend/append` were also passed --- src/components/form/field_number/field_number.tsx | 6 ++++-- src/components/form/field_text/field_text.tsx | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/components/form/field_number/field_number.tsx b/src/components/form/field_number/field_number.tsx index b535aca00301..9b8bc28569a4 100644 --- a/src/components/form/field_number/field_number.tsx +++ b/src/components/form/field_number/field_number.tsx @@ -146,10 +146,12 @@ export const EuiFieldNumber: FunctionComponent = ( }); const classes = classNames('euiFieldNumber', className, numIconsClass, { - 'euiFieldNumber--withIcon': icon && !hasRightSideIcon, 'euiFieldNumber--fullWidth': fullWidth, 'euiFieldNumber--compressed': compressed, - 'euiFieldNumber--inGroup': prepend || append, + ...(!controlOnly && { + 'euiFieldNumber--inGroup': prepend || append, + 'euiFieldNumber--withIcon': icon && !hasRightSideIcon, + }), 'euiFieldNumber-isLoading': isLoading, }); diff --git a/src/components/form/field_text/field_text.tsx b/src/components/form/field_text/field_text.tsx index 01a83f55d718..fb89e2263b4a 100644 --- a/src/components/form/field_text/field_text.tsx +++ b/src/components/form/field_text/field_text.tsx @@ -92,10 +92,12 @@ export const EuiFieldText: FunctionComponent = (props) => { }); const classes = classNames('euiFieldText', className, numIconsClass, { - 'euiFieldText--withIcon': icon && !hasRightSideIcon, 'euiFieldText--fullWidth': fullWidth, 'euiFieldText--compressed': compressed, - 'euiFieldText--inGroup': prepend || append, + ...(!controlOnly && { + 'euiFieldText--withIcon': icon && !hasRightSideIcon, + 'euiFieldText--inGroup': prepend || append, + }), 'euiFieldText-isLoading': isLoading, }); From d375b56b86b486fcdf6640f76dd05f8348435233 Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 13:34:32 -0700 Subject: [PATCH 6/7] [EuiColorPicker] Fix input padding - icon behavior was previously relying on buggy `EuiFieldText` logic - also didn't have the correct right side padding CSS in any case --- .../__snapshots__/color_picker.test.tsx.snap | 24 +++++++++---------- .../color_picker/_color_picker.scss | 16 ------------- src/components/color_picker/color_picker.tsx | 15 ++++++++++-- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/src/components/color_picker/__snapshots__/color_picker.test.tsx.snap b/src/components/color_picker/__snapshots__/color_picker.test.tsx.snap index 3a3b711332e9..01a480b67046 100644 --- a/src/components/color_picker/__snapshots__/color_picker.test.tsx.snap +++ b/src/components/color_picker/__snapshots__/color_picker.test.tsx.snap @@ -27,7 +27,7 @@ exports[`renders EuiColorPicker 1`] = ` = ({ 'euiColorPicker__popoverPanel--customButton': button, }); const swatchClass = 'euiColorPicker__swatchSelect'; - const inputClasses = classNames('euiColorPicker__input', { - 'euiColorPicker__input--inGroup': prepend || append, + + const numIconsClass = getFormControlClassNameForIconCount({ + isDropdown: true, + clear: isClearable, + isInvalid, }); + const inputClasses = classNames( + 'euiColorPicker__input', + { 'euiColorPicker__input--inGroup': prepend || append }, + // Manually account for input padding, since `controlOnly` disables that logic + 'euiFieldText--withIcon', + numIconsClass + ); const handleOnChange = (text: string) => { const output = getOutput(text, showAlpha); From 71d996336fc0e71ca1e07913827fbe677a2f4c7e Mon Sep 17 00:00:00 2001 From: Cee Chen Date: Tue, 9 Apr 2024 13:41:18 -0700 Subject: [PATCH 7/7] changelogs --- changelogs/upcoming/7666.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelogs/upcoming/7666.md diff --git a/changelogs/upcoming/7666.md b/changelogs/upcoming/7666.md new file mode 100644 index 000000000000..ea0f6ffa1fb3 --- /dev/null +++ b/changelogs/upcoming/7666.md @@ -0,0 +1,6 @@ +**Bug fixes** + +- Fixed `EuiFieldNumber`'s typing to accept an icon configuration shape +- Fixed `EuiFieldText` and `EuiFieldNumber` to render the correct paddings for icon shapes set to `side: 'right'` +- Fixed `EuiFieldText` and `EuiFieldNumber` to fully ignore `icon`/`prepend`/`append` when `controlOnly` is set to true +- Fixed `EuiColorPicker`'s input not setting the correct right padding for the number of icons displayed