fix(many): fix regression where form elements without label got misaligned

balzss · balzss · commit 139f7f130cd7 · 2024-11-11T13:48:06.000+01:00
diff --git a/docs/guides/form-errors.md b/docs/guides/form-errors.md
@@ -51,9 +51,9 @@ However you might have noticed from the type definition that a message can be `e
 
 We wanted to allow users to start using the new format without making it mandatory, but after the introductory period `newError` will be deprecated and `error` type will be changed to look and behave the same way.
 
-With this update we also introduced the "required asterisk" which will display an `*` character next to field labels that are required. This update is not opt-in and will apply to **all** InstUI form components so if you we relying on a custom solution for this feature before, you need to remove that to avoid having double asterisks.
+With this update we also introduced the "required asterisk" which will display an `*` character next to field labels that are required. This update is not opt-in and will apply to **all** InstUI form components so if you were relying on a custom solution for this feature before, you need to remove that to avoid having double asterisks.
 
-Here are examples with different form components: 
+Here are examples with different form components:
 
 ```ts
 ---
@@ -98,7 +98,7 @@ const Example = () => {
         <TextArea messages={messages} label="TextArea" required={isRequired}/>
 
         <Checkbox label="Checkbox" isRequired={isRequired} messages={messages}/>
- 
+
         <Checkbox label={`Checkbox (variant="toggle")`} variant="toggle" isRequired={isRequired} messages={messages}/>
 
         <CheckboxGroup
@@ -127,6 +127,14 @@ const Example = () => {
         </RadioInputGroup>
 
         <FileDrop messages={messages} renderLabel="FileDrop" />
+
+        <ColorPicker
+          label="ColorPicker"
+          placeholderText="Enter HEX"
+          isRequired={isRequired}
+          renderMessages={() => messages}
+        />
+
       </div>
     </div>
   )
diff --git a/packages/ui-checkbox/src/Checkbox/index.tsx b/packages/ui-checkbox/src/Checkbox/index.tsx
@@ -230,7 +230,7 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
           invalid={this.invalid}
         >
           {label}
-          {isRequired && (
+          {isRequired && label && (
             <span css={this.invalid ? styles?.requiredInvalid : {}}> *</span>
           )}
         </ToggleFacade>
@@ -247,7 +247,7 @@ class Checkbox extends Component<CheckboxProps, CheckboxState> {
           invalid={this.invalid}
         >
           {label}
-          {isRequired && (
+          {isRequired && label && (
             <span css={this.invalid ? styles?.requiredInvalid : {}}> *</span>
           )}
         </CheckboxFacade>
diff --git a/packages/ui-checkbox/src/Checkbox/props.ts b/packages/ui-checkbox/src/Checkbox/props.ts
@@ -44,7 +44,7 @@ type CheckboxOwnProps = {
   /**
    * Array of objects with shape: `{
    *   text: ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    *   }`
    */
   messages?: FormMessage[]
diff --git a/packages/ui-color-picker/src/ColorPicker/index.tsx b/packages/ui-color-picker/src/ColorPicker/index.tsx
@@ -268,7 +268,7 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
       isRequiredMessages =
         typeof renderIsRequiredMessage === 'function'
           ? renderIsRequiredMessage()
-          : [{ type: 'error', text: '*' }]
+          : [{ type: 'error', text: '' }]
     }
     if (typeof renderMessages === 'function') {
       generalMessages = renderMessages(
@@ -386,14 +386,14 @@ class ColorPicker extends Component<ColorPickerProps, ColorPickerState> {
     const { label, tooltip, styles } = this.props
 
     return tooltip ? (
-      <div>
+      <span>
         <span css={styles?.label}>{label}</span>
         <span>
           <Tooltip renderTip={tooltip}>
             <IconInfoLine tabIndex={-1} />
           </Tooltip>
         </span>
-      </div>
+      </span>
     ) : (
       label
     )
diff --git a/packages/ui-color-picker/src/ColorPicker/props.ts b/packages/ui-color-picker/src/ColorPicker/props.ts
@@ -52,7 +52,7 @@ type ColorPickerOwnProps = {
    *
    * FormMessage[]: Array of objects with shape: `{
    *   text: ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   checkContrast?: {
@@ -164,7 +164,7 @@ type ColorPickerOwnProps = {
    *
    * FormMessage[]: Array of objects with shape: `{
    *   text: ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   renderInvalidColorMessage?: (hexCode: string) => FormMessage[]
@@ -174,7 +174,7 @@ type ColorPickerOwnProps = {
    *
    * FormMessage[]: Array of objects with shape: `{
    *   text: ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   renderIsRequiredMessage?: () => FormMessage[]
@@ -184,7 +184,7 @@ type ColorPickerOwnProps = {
    *
    * FormMessage[]: Array of objects with shape: `{
    *   text: ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   renderMessages?: (
diff --git a/packages/ui-form-field/src/FormField/props.ts b/packages/ui-form-field/src/FormField/props.ts
@@ -43,7 +43,7 @@ type FormFieldOwnProps = {
   /**
    * Array of objects with shape: `{
    *   text: React.ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   messages?: FormMessage[]
diff --git a/packages/ui-form-field/src/FormFieldGroup/props.ts b/packages/ui-form-field/src/FormFieldGroup/props.ts
@@ -45,7 +45,7 @@ type FormFieldGroupOwnProps = {
   /**
    * Array of objects with shape: `{
    *   text: React.ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   messages?: FormMessage[]
diff --git a/packages/ui-form-field/src/FormFieldLayout/props.ts b/packages/ui-form-field/src/FormFieldLayout/props.ts
@@ -48,7 +48,7 @@ type FormFieldLayoutOwnProps = {
   /**
    * Array of objects with shape: `{
    *   text: React.ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   messages?: FormMessage[]
@@ -79,7 +79,9 @@ type FormFieldLayoutProps = FormFieldLayoutOwnProps &
   OtherHTMLAttributes<FormFieldLayoutOwnProps> &
   WithDeterministicIdProps
 
-type FormFieldLayoutStyle = ComponentStyle<'formFieldLayout' | 'groupErrorMessage'>
+type FormFieldLayoutStyle = ComponentStyle<
+  'formFieldLayout' | 'groupErrorMessage'
+>
 
 const propTypes: PropValidators<PropKeys> = {
   label: PropTypes.node.isRequired,
diff --git a/packages/ui-form-field/src/FormFieldLayout/styles.ts b/packages/ui-form-field/src/FormFieldLayout/styles.ts
@@ -42,7 +42,7 @@ const generateStyle = (
 
   return {
     groupErrorMessage: {
-      margin: '0.5rem',
+      margin: '0.5rem 0',
     },
     formFieldLayout: {
       label: 'formFieldLayout',
diff --git a/packages/ui-form-field/src/FormFieldMessages/props.ts b/packages/ui-form-field/src/FormFieldMessages/props.ts
@@ -38,7 +38,7 @@ type FormFieldMessagesOwnProps = {
   /**
    * Array of objects with shape: `{
    *   text: React.ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   messages?: FormMessage[]
diff --git a/packages/ui-number-input/src/NumberInput/index.tsx b/packages/ui-number-input/src/NumberInput/index.tsx
@@ -39,6 +39,7 @@ import {
   getInteraction,
   withDeterministicId
 } from '@instructure/ui-react-utils'
+import { hasVisibleChildren } from '@instructure/ui-a11y-utils'
 
 import { withStyle, jsx } from '@instructure/emotion'
 
@@ -101,7 +102,9 @@ class NumberInput extends Component<NumberInputProps, NumberInputState> {
   get invalid() {
     return (
       !!this.props.messages &&
-      this.props.messages.some((message) => message.type === 'error' || message.type === 'newError')
+      this.props.messages.some(
+        (message) => message.type === 'error' || message.type === 'newError'
+      )
     )
   }
 
@@ -238,17 +241,22 @@ class NumberInput extends Component<NumberInputProps, NumberInputState> {
 
     const { interaction } = this
 
+    const rawLabel = callRenderProp(renderLabel)
+    const label = hasVisibleChildren(rawLabel) ? (
+      <React.Fragment>
+        {rawLabel}
+        {isRequired && (
+          <span css={this.invalid ? styles?.requiredInvalid : {}}> *</span>
+        )}
+      </React.Fragment>
+    ) : (
+      rawLabel
+    )
+
     return (
       <FormField
         {...pickProps(this.props, FormField.allowedProps)}
-        label={
-          <React.Fragment>
-            {callRenderProp(renderLabel)}
-            {isRequired && (
-              <span css={this.invalid ? styles?.requiredInvalid : {}}> *</span>
-            )}
-          </React.Fragment>
-        }
+        label={label}
         inline={display === 'inline-block'}
         id={this.id}
         elementRef={this.handleRef}
diff --git a/packages/ui-number-input/src/NumberInput/props.ts b/packages/ui-number-input/src/NumberInput/props.ts
@@ -64,7 +64,7 @@ type NumberInputOwnProps = {
   /**
    * Array of objects with shape: `{
    *   text: ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   messages?: FormMessage[]
diff --git a/packages/ui-radio-input/src/RadioInputGroup/index.tsx b/packages/ui-radio-input/src/RadioInputGroup/index.tsx
@@ -34,6 +34,7 @@ import {
   pickProps,
   withDeterministicId
 } from '@instructure/ui-react-utils'
+import { hasVisibleChildren } from '@instructure/ui-a11y-utils'
 
 import { RadioInput } from '../RadioInput'
 import type { RadioInputProps } from '../RadioInput/props'
@@ -160,13 +161,15 @@ class RadioInputGroup extends Component<
   render() {
     const { variant, layout, description, isRequired, styles } = this.props
 
-    const descriptionWithRequired = (
+    const descriptionWithRequired = hasVisibleChildren(description) ? (
       <React.Fragment>
         {description}
-        {isRequired && (
+        {isRequired && description && (
           <span css={this.invalid ? styles?.invalidAsterisk : {}}> *</span>
         )}
       </React.Fragment>
+    ) : (
+      description
     )
 
     return (
diff --git a/packages/ui-select/src/Select/props.ts b/packages/ui-select/src/Select/props.ts
@@ -202,7 +202,7 @@ type PropsFromTextInput = {
    * with the following shape:
    * `{
    *   text: React.ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   messages?: FormMessage[]
diff --git a/packages/ui-simple-select/src/SimpleSelect/props.ts b/packages/ui-simple-select/src/SimpleSelect/props.ts
@@ -167,7 +167,7 @@ type PropsPassedToSelect = {
    * objects with the following shape:
    * `{
    *   text: ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   messages?: FormMessage[]
diff --git a/packages/ui-text-area/src/TextArea/index.tsx b/packages/ui-text-area/src/TextArea/index.tsx
@@ -42,6 +42,7 @@ import {
   pickProps,
   withDeterministicId
 } from '@instructure/ui-react-utils'
+import { hasVisibleChildren } from '@instructure/ui-a11y-utils'
 
 import generateStyle from './styles'
 import generateComponentTheme from './theme'
@@ -314,7 +315,7 @@ class TextArea extends Component<TextAreaProps> {
       maxHeight,
       textareaRef,
       resize,
-      styles,
+      styles
     } = this.props
 
     const props = omitProps(this.props, TextArea.allowedProps)
@@ -353,17 +354,21 @@ class TextArea extends Component<TextAreaProps> {
       />
     )
 
+    const label = hasVisibleChildren(this.props.label) ? (
+      <React.Fragment>
+        {this.props.label}
+        {required && (
+          <span css={this.invalid ? styles?.requiredInvalid : {}}> *</span>
+        )}
+      </React.Fragment>
+    ) : (
+      this.props.label
+    )
+
     return (
       <FormField
         {...pickProps(this.props, FormField.allowedProps)}
-        label={
-          <React.Fragment>
-            {this.props.label}
-            {required && (
-              <span css={this.invalid ? styles?.requiredInvalid : {}}> *</span>
-            )}
-          </React.Fragment>
-        }
+        label={label}
         vAlign="top"
         id={this.id}
         elementRef={(el) => {
diff --git a/packages/ui-text-area/src/TextArea/props.ts b/packages/ui-text-area/src/TextArea/props.ts
@@ -72,7 +72,7 @@ type TextAreaOwnProps = {
   /**
    * Array of objects with shape: `{
    *   text: React.ReactNode,
-   *   type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *   type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   messages?: FormMessage[]
diff --git a/packages/ui-text-input/src/TextInput/index.tsx b/packages/ui-text-input/src/TextInput/index.tsx
@@ -35,6 +35,7 @@ import { isActiveElement, addEventListener } from '@instructure/ui-dom-utils'
 import { FormField } from '@instructure/ui-form-field'
 import { testable } from '@instructure/ui-testable'
 import { withStyle, jsx } from '@instructure/emotion'
+import { hasVisibleChildren } from '@instructure/ui-a11y-utils'
 
 import generateStyle from './styles'
 import generateComponentTheme from './theme'
@@ -327,17 +328,22 @@ class TextInput extends Component<TextInputProps, TextInputState> {
 
     const renderBeforeOrAfter = !!beforeElement || !!afterElement
 
+    const rawLabel = callRenderProp(renderLabel)
+    const label = hasVisibleChildren(rawLabel) ? (
+      <React.Fragment>
+        {rawLabel}
+        {isRequired && (
+          <span css={this.invalid ? styles?.requiredInvalid : {}}> *</span>
+        )}
+      </React.Fragment>
+    ) : (
+      rawLabel
+    )
+
     return (
       <FormField
         id={this.id}
-        label={
-          <React.Fragment>
-            {callRenderProp(renderLabel)}
-            {isRequired && (
-              <span css={this.invalid ? styles?.requiredInvalid : {}}> *</span>
-            )}
-          </React.Fragment>
-        }
+        label={label}
         messagesId={this._messagesId}
         messages={messages}
         inline={display === 'inline-block'}
diff --git a/packages/ui-time-select/src/TimeSelect/props.ts b/packages/ui-time-select/src/TimeSelect/props.ts
@@ -127,7 +127,7 @@ type TimeSelectOwnProps = {
    * objects with the following shape:
    * `{
    *     text: ReactNode,
-   *     type: One of: ['error', 'hint', 'success', 'screenreader-only']
+   *     type: One of: ['newError', 'error', 'hint', 'success', 'screenreader-only']
    * }`
    */
   messages?: FormMessage[]
