From ee9cafdd027b9a1caaa0791d1b6dc4f8401c87e7 Mon Sep 17 00:00:00 2001 From: Matyas Szabo Date: Fri, 6 Dec 2024 16:39:53 +0100 Subject: [PATCH] fix(ui-select,ui-text-input): fix long before elements overflowing in TextInput, Select, SimpleSelect Closes: INSTUI-4344 When adding lots of elements to renderBeforeInput for example Tags it was overflowing the input field. Also when there are more than one line of elements keep the input field in the same line if there is enough space TEST PLAN: Add lots of elements to renderBeforeInput to Select,SimpleSelect,TextInput. They should wrap, not overflow --- .../src/FormFieldLayout/index.tsx | 14 +---- packages/ui-react-utils/src/omitProps.ts | 3 +- packages/ui-select/src/Select/README.md | 8 ++- .../ui-text-input/src/TextInput/README.md | 13 +++-- .../ui-text-input/src/TextInput/index.tsx | 55 +++++-------------- packages/ui-text-input/src/TextInput/props.ts | 7 +-- .../ui-text-input/src/TextInput/styles.ts | 42 ++++---------- 7 files changed, 47 insertions(+), 95 deletions(-) diff --git a/packages/ui-form-field/src/FormFieldLayout/index.tsx b/packages/ui-form-field/src/FormFieldLayout/index.tsx index cf7c810c6c..13de6eaa07 100644 --- a/packages/ui-form-field/src/FormFieldLayout/index.tsx +++ b/packages/ui-form-field/src/FormFieldLayout/index.tsx @@ -28,7 +28,6 @@ import { Component } from 'react' import { hasVisibleChildren } from '@instructure/ui-a11y-utils' import { ScreenReaderContent } from '@instructure/ui-a11y-content' import { Grid } from '@instructure/ui-grid' -import { logError as error } from '@instructure/console' import { omitProps, pickProps, @@ -67,16 +66,7 @@ class FormFieldLayout extends Component { constructor(props: FormFieldLayoutProps) { super(props) - this._messagesId = props.messagesId || props.deterministicId!() - - error( - typeof props.width !== 'undefined' || - !props.inline || - props.layout !== 'inline', - `[FormFieldLayout] The 'inline' prop is true, and the 'layout' is set to 'inline'. - This will cause a layout issue in Internet Explorer 11 unless you also add a value for the 'width' prop.` - ) } private _messagesId: string @@ -212,7 +202,9 @@ class FormFieldLayout extends Component { elementRef={this.handleInputContainerRef} > {hasNewErrorMsg && ( -
{this.renderVisibleMessages()}
+
+ {this.renderVisibleMessages()} +
)} {children} diff --git a/packages/ui-react-utils/src/omitProps.ts b/packages/ui-react-utils/src/omitProps.ts index 173b2ffc82..8b519687a8 100644 --- a/packages/ui-react-utils/src/omitProps.ts +++ b/packages/ui-react-utils/src/omitProps.ts @@ -29,7 +29,8 @@ * Return an object with the remaining props after the given props are omitted. * * Automatically excludes the following props: - * 'theme', 'children', 'className', 'style', 'styles', 'makeStyles', 'themeOverride', 'deterministicId' + * `theme`, `children`, `className`, `style`, `styles`, `makeStyles`, + * `themeOverride`, `deterministicId` * @module omitProps * @param props The object to process * @param propsToOmit list disallowed prop keys or an object whose diff --git a/packages/ui-select/src/Select/README.md b/packages/ui-select/src/Select/README.md index b8cd55dd90..2ff16e3060 100644 --- a/packages/ui-select/src/Select/README.md +++ b/packages/ui-select/src/Select/README.md @@ -850,7 +850,9 @@ To mark an option as "highlighted", use the option's `isHighlighted` prop. Note {this.getOptionById(id).label} } - margin={index > 0 ? 'xxx-small 0 xxx-small xx-small' : 'xxx-small 0'} + margin={ + index > 0 ? 'xxx-small xx-small xxx-small 0' : '0 xx-small 0 0' + } onClick={(e) => this.dismissTag(e, id)} /> )) @@ -1079,7 +1081,9 @@ To mark an option as "highlighted", use the option's `isHighlighted` prop. Note {this.getOptionById(id).label} } - margin={index > 0 ? 'xxx-small 0 xxx-small xx-small' : 'xxx-small 0'} + margin={ + index > 0 ? 'xxx-small xx-small xxx-small 0' : '0 xx-small 0 0' + } onClick={(e) => dismissTag(e, id)} /> )) diff --git a/packages/ui-text-input/src/TextInput/README.md b/packages/ui-text-input/src/TextInput/README.md index e7b26c3e62..3e1330f702 100644 --- a/packages/ui-text-input/src/TextInput/README.md +++ b/packages/ui-text-input/src/TextInput/README.md @@ -205,7 +205,7 @@ Focusable content will be focused separately from the input itself. value={this.state.value} onChange={this.handleChange} renderBeforeInput={ - + <> {this.state.value !== '' && ( console.log('Strawberry')} /> - + } renderAfterInput={() => ( @@ -260,7 +260,7 @@ Focusable content will be focused separately from the input itself. value={value} onChange={handleChange} renderBeforeInput={ - + <> {value !== '' && ( console.log('Strawberry')} /> - + } renderAfterInput={() => ( @@ -346,7 +346,7 @@ type: example + <> - + } renderAfterInput={} /> @@ -374,6 +374,7 @@ type: embed Left align text (exceptions may apply) Place labels on top or to the left (inline) Make placeholder text different than the label + Use React fragments for renderBeforeInput. This will nicely float the text input box to the remaining space
Place labels to the right of the input diff --git a/packages/ui-text-input/src/TextInput/index.tsx b/packages/ui-text-input/src/TextInput/index.tsx index 137484dc0a..5a208d45b3 100644 --- a/packages/ui-text-input/src/TextInput/index.tsx +++ b/packages/ui-text-input/src/TextInput/index.tsx @@ -77,7 +77,6 @@ class TextInput extends Component { super(props) this.state = { hasFocus: false, - beforeElementHasWidth: undefined, afterElementHasWidth: undefined } this._defaultId = props.deterministicId!() @@ -87,7 +86,6 @@ class TextInput extends Component { ref: Element | null = null private _input: HTMLInputElement | null = null - private _beforeElement: HTMLSpanElement | null = null private _afterElement: HTMLSpanElement | null = null private readonly _defaultId: string @@ -112,13 +110,10 @@ class TextInput extends Component { 'focus', this.handleFocus ) - this.setState({ - beforeElementHasWidth: this.getElementHasWidth(this._beforeElement), afterElementHasWidth: this.getElementHasWidth(this._afterElement) }) } - this.props.makeStyles?.(this.makeStyleProps()) } @@ -129,11 +124,6 @@ class TextInput extends Component { } componentDidUpdate(prevProps: TextInputProps) { - if (prevProps.renderBeforeInput !== this.props.renderBeforeInput) { - this.setState({ - beforeElementHasWidth: this.getElementHasWidth(this._beforeElement) - }) - } if (prevProps.renderAfterInput !== this.props.renderAfterInput) { this.setState({ afterElementHasWidth: this.getElementHasWidth(this._afterElement) @@ -154,13 +144,13 @@ class TextInput extends Component { makeStyleProps = (): TextInputStyleProps => { const { interaction } = this - const { hasFocus, beforeElementHasWidth, afterElementHasWidth } = this.state + const { hasFocus, afterElementHasWidth } = this.state return { disabled: interaction === 'disabled', invalid: this.invalid, focused: hasFocus, - beforeElementHasWidth, - afterElementHasWidth + afterElementHasWidth: afterElementHasWidth, + beforeElementExists: this.props.renderBeforeInput != undefined } } @@ -282,7 +272,7 @@ class TextInput extends Component { ) } - getElementHasWidth(element: HTMLSpanElement | null) { + getElementHasWidth(element: Element | null) { if (!element) { return undefined } @@ -360,39 +350,24 @@ class TextInput extends Component { > {renderBeforeOrAfter ? ( -
- - {beforeElement && ( + + {beforeElement} + {/* The input and content after input should not wrap, + so they're in their own flex container */} + + {this.renderInput()} + {afterElement && ( { - this._beforeElement = e + this._afterElement = e }} > - {beforeElement} + {afterElement} )} - - {/* - The input and content after input should not wrap, - so they're in their own flex container - */} - - {this.renderInput()} - {afterElement && ( - { - this._afterElement = e - }} - > - {afterElement} - - )} - - -
+ ) : ( /* If no prepended or appended content, don't render Flex layout */ this.renderInput() diff --git a/packages/ui-text-input/src/TextInput/props.ts b/packages/ui-text-input/src/TextInput/props.ts index ab3c64ba03..781accdf85 100644 --- a/packages/ui-text-input/src/TextInput/props.ts +++ b/packages/ui-text-input/src/TextInput/props.ts @@ -193,11 +193,9 @@ type TextInputStyle = ComponentStyle< | 'textInput' | 'facade' | 'layout' - | 'beforeElement' - | 'innerWrapper' - | 'inputLayout' | 'afterElement' | 'requiredInvalid' + | 'inputLayout' > const propTypes: PropValidators = { @@ -254,7 +252,6 @@ const allowedProps: AllowedPropKeys = [ type TextInputState = { hasFocus: boolean - beforeElementHasWidth?: boolean afterElementHasWidth?: boolean } @@ -262,8 +259,8 @@ type TextInputStyleProps = { disabled: boolean invalid: boolean focused: TextInputState['hasFocus'] - beforeElementHasWidth: TextInputState['beforeElementHasWidth'] afterElementHasWidth: TextInputState['afterElementHasWidth'] + beforeElementExists: boolean } export type { diff --git a/packages/ui-text-input/src/TextInput/styles.ts b/packages/ui-text-input/src/TextInput/styles.ts index db011516d0..7437b1542a 100644 --- a/packages/ui-text-input/src/TextInput/styles.ts +++ b/packages/ui-text-input/src/TextInput/styles.ts @@ -49,8 +49,8 @@ const generateStyle = ( disabled, invalid, focused, - beforeElementHasWidth, - afterElementHasWidth + afterElementHasWidth, + beforeElementExists } = state const sizeVariants = { @@ -103,15 +103,14 @@ const generateStyle = ( const inputStyle = { all: 'initial', - '&::-ms-clear': { display: 'none' }, + width: '100%', WebkitFontSmoothing: 'antialiased', MozOsxFontSmoothing: 'grayscale', appearance: 'none', margin: 0, - width: '100%', display: 'block', boxSizing: 'border-box', outline: 'none', @@ -151,11 +150,6 @@ const generateStyle = ( flexDirection: 'row' } - const flexItemBase = { - ...viewBase, - flexShrink: 0 - } - return { requiredInvalid: { color: componentTheme.requiredInvalidColor @@ -198,30 +192,17 @@ const generateStyle = ( }, layout: { label: 'textInput__layout', - ...flexBase, - ...(!shouldNotWrap && { flexWrap: 'wrap' }) - }, - beforeElement: { - display: 'inline-flex', + ...viewBase, + display: 'flex', alignItems: 'center', - label: 'textInput__beforeElement', - ...flexItemBase, - paddingInlineStart: componentTheme.padding, - // we only override the padding once the width is calculated, - // it needs the padding on render - ...(beforeElementHasWidth === false && { - paddingInlineStart: 0 - }) - }, - innerWrapper: { - label: 'textInput__innerWrapper', - ...flexItemBase, - minWidth: '0.0625rem', - flexShrink: 1, - flexGrow: 1 + justifyContent: 'flex-start', + flexDirection: 'row', + ...(!shouldNotWrap && { flexWrap: 'wrap' }), + ...(beforeElementExists && { paddingInlineStart: componentTheme.padding }) }, inputLayout: { label: 'textInput__inputLayout', + flexGrow: 1, ...flexBase }, afterElement: { @@ -231,7 +212,8 @@ const generateStyle = ( marginTop: '-1px', marginBottom: '-1px', label: 'textInput__afterElement', - ...flexItemBase, + ...viewBase, + flexShrink: 0, paddingInlineEnd: componentTheme.padding, // we only override the padding once the width is calculated, // it needs the padding on render