From 9b0e1daebc10938fd84517c57fbd99baf8426d88 Mon Sep 17 00:00:00 2001 From: Cee Chen <549407+cee-chen@users.noreply.github.com> Date: Thu, 11 Apr 2024 09:10:36 -0700 Subject: [PATCH] [EuiFieldText][EuiFieldNumber] Various `icon` related fixes (#7666) --- changelogs/upcoming/7666.md | 6 ++ .../__snapshots__/color_picker.test.tsx.snap | 24 +++---- .../color_picker/_color_picker.scss | 16 ----- src/components/color_picker/color_picker.tsx | 15 ++++- .../field_number/field_number.stories.tsx | 62 ++++++++++++++++++ .../form/field_number/field_number.tsx | 16 +++-- .../form/field_text/field_text.stories.tsx | 64 +++++++++++++++++++ src/components/form/field_text/field_text.tsx | 14 +++- .../form_control_layout/_num_icons.test.ts | 29 ++++++++- .../form/form_control_layout/_num_icons.ts | 17 ++++- .../form_control_layout_icons.tsx | 6 +- 11 files changed, 224 insertions(+), 45 deletions(-) create mode 100644 changelogs/upcoming/7666.md create mode 100644 src/components/form/field_text/field_text.stories.tsx diff --git a/changelogs/upcoming/7666.md b/changelogs/upcoming/7666.md new file mode 100644 index 00000000000..ea0f6ffa1fb --- /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 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 3a3b711332e..01a480b6704 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); diff --git a/src/components/form/field_number/field_number.stories.tsx b/src/components/form/field_number/field_number.stories.tsx index b283f0d8e5f..0aa0ab52b7e 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_number/field_number.tsx b/src/components/form/field_number/field_number.tsx index f7a6f9e003f..9b8bc28569a 100644 --- a/src/components/form/field_number/field_number.tsx +++ b/src/components/form/field_number/field_number.tsx @@ -19,14 +19,16 @@ import classNames from 'classnames'; import { useCombinedRefs } from '../../../services'; import { CommonProps } from '../../common'; -import { IconType } from '../../icon'; import { EuiValidatableControl } from '../validatable_control'; 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< @@ -34,7 +36,7 @@ export type EuiFieldNumberProps = Omit< 'min' | 'max' | 'readOnly' | 'step' > & CommonProps & { - icon?: IconType; + icon?: EuiFormControlLayoutProps['icon']; isInvalid?: boolean; /** * Expand to fill 100% of the parent. @@ -134,18 +136,22 @@ 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--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.stories.tsx b/src/components/form/field_text/field_text.stories.tsx new file mode 100644 index 00000000000..4484f691814 --- /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']); diff --git a/src/components/form/field_text/field_text.tsx b/src/components/form/field_text/field_text.tsx index 3a40234d5ed..fb89e2263b4 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,18 +81,23 @@ 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--fullWidth': fullWidth, 'euiFieldText--compressed': compressed, - 'euiFieldText--inGroup': prepend || append, + ...(!controlOnly && { + 'euiFieldText--withIcon': icon && !hasRightSideIcon, + 'euiFieldText--inGroup': prepend || append, + }), 'euiFieldText-isLoading': isLoading, }); 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 ae56dd29629..4b3f26693e7 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 e9a15884e08..95c792c4cf2 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 e8d23bd2fc3..5cfd7ce1ac3 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;