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
instructure · Dec 6, 2024 · ee9cafd · ee9cafd
1 parent ef3e930
commit ee9cafd
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 95 deletions.
diff --git 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<FormFieldLayoutProps> {
 
   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<FormFieldLayoutProps> {
               elementRef={this.handleInputContainerRef}
             >
               {hasNewErrorMsg && (
-                <div css={styles?.groupErrorMessage}>{this.renderVisibleMessages()}</div>
+                <div css={styles?.groupErrorMessage}>
+                  {this.renderVisibleMessages()}
+                </div>
               )}
               {children}
             </Grid.Col>

diff --git 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
@@ -850,7 +850,9 @@ To mark an option as "highlighted", use the option's `isHighlighted` prop. Note
               {this.getOptionById(id).label}
             </AccessibleContent>
           }
-          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}
             </AccessibleContent>
           }
-          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
@@ -205,7 +205,7 @@ Focusable content will be focused separately from the input itself.
             value={this.state.value}
             onChange={this.handleChange}
             renderBeforeInput={
-              <View display="block" padding="xxx-small 0">
+              <>
                 {this.state.value !== '' && (
                   <Tag
                     text={this.state.value}
@@ -233,7 +233,7 @@ Focusable content will be focused separately from the input itself.
                   margin="xxx-small xxx-small xxx-small none"
                   onClick={() => console.log('Strawberry')}
                 />
-              </View>
+              </>
             }
             renderAfterInput={() => (
               <Avatar name="Paula Panda" src={avatarSquare} size="x-small" />
@@ -260,7 +260,7 @@ Focusable content will be focused separately from the input itself.
           value={value}
           onChange={handleChange}
           renderBeforeInput={
-            <View display="block" padding="xxx-small 0">
+            <>
               {value !== '' && (
                 <Tag
                   text={value}
@@ -288,7 +288,7 @@ Focusable content will be focused separately from the input itself.
                 margin="xxx-small xxx-small xxx-small none"
                 onClick={() => console.log('Strawberry')}
               />
-            </View>
+            </>
           }
           renderAfterInput={() => (
             <Avatar name="Paula Panda" src={avatarSquare} size="x-small" />
@@ -346,7 +346,7 @@ type: example
     <TextInput
       renderLabel="I will wrap"
       renderBeforeInput={
-        <div>
+        <>
           <Tag
             text="English 101"
             margin="xx-small xxx-small"
@@ -355,7 +355,7 @@ type: example
             text="History 205"
             margin="xx-small xxx-small"
           />
-        </div>
+        </>
       }
       renderAfterInput={<Avatar name="Paula Panda" src={avatarSquare} size="x-small" />}
     />
@@ -374,6 +374,7 @@ type: embed
     <Figure.Item>Left align text (exceptions may apply)</Figure.Item>
     <Figure.Item>Place labels on top or to the left (inline)</Figure.Item>
     <Figure.Item>Make placeholder text different than the label</Figure.Item>
+    <Figure.Item>Use React fragments for <code>renderBeforeInput</code>. This will nicely float the text input box to the remaining space</Figure.Item>
   </Figure>
   <Figure recommendation="no" title="Don't">
     <Figure.Item>Place labels to the right of the input</Figure.Item>

diff --git 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<TextInputProps, TextInputState> {
     super(props)
     this.state = {
       hasFocus: false,
-      beforeElementHasWidth: undefined,
       afterElementHasWidth: undefined
     }
     this._defaultId = props.deterministicId!()
@@ -87,7 +86,6 @@ class TextInput extends Component<TextInputProps, TextInputState> {
   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<TextInputProps, TextInputState> {
         '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<TextInputProps, TextInputState> {
   }
 
   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<TextInputProps, TextInputState> {
 
   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<TextInputProps, TextInputState> {
     )
   }
 
-  getElementHasWidth(element: HTMLSpanElement | null) {
+  getElementHasWidth(element: Element | null) {
     if (!element) {
       return undefined
     }
@@ -360,39 +350,24 @@ class TextInput extends Component<TextInputProps, TextInputState> {
       >
         <span css={styles?.facade}>
           {renderBeforeOrAfter ? (
-            <div>
-              <span css={styles?.layout}>
-                {beforeElement && (
+            <span css={styles?.layout}>
+              {beforeElement}
+              {/* The input and content after input should not wrap,
+                so they're in their own flex container */}
+              <span css={styles?.inputLayout}>
+                {this.renderInput()}
+                {afterElement && (
                   <span
-                    css={styles?.beforeElement}
+                    css={styles?.afterElement}
                     ref={(e) => {
-                      this._beforeElement = e
+                      this._afterElement = e
                     }}
                   >
-                    {beforeElement}
+                    {afterElement}
                   </span>
                 )}
-                <span css={styles?.innerWrapper}>
-                  {/*
-                    The input and content after input should not wrap,
-                    so they're in their own flex container
-                  */}
-                  <span css={styles?.inputLayout}>
-                    <span css={styles?.innerWrapper}>{this.renderInput()}</span>
-                    {afterElement && (
-                      <span
-                        css={styles?.afterElement}
-                        ref={(e) => {
-                          this._afterElement = e
-                        }}
-                      >
-                        {afterElement}
-                      </span>
-                    )}
-                  </span>
-                </span>
               </span>
-            </div>
+            </span>
           ) : (
             /* 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
@@ -193,11 +193,9 @@ type TextInputStyle = ComponentStyle<
   | 'textInput'
   | 'facade'
   | 'layout'
-  | 'beforeElement'
-  | 'innerWrapper'
-  | 'inputLayout'
   | 'afterElement'
   | 'requiredInvalid'
+  | 'inputLayout'
 >
 
 const propTypes: PropValidators<PropKeys> = {
@@ -254,16 +252,15 @@ const allowedProps: AllowedPropKeys = [
 
 type TextInputState = {
   hasFocus: boolean
-  beforeElementHasWidth?: boolean
   afterElementHasWidth?: boolean
 }
 
 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
@@ -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