fix(ui): fix row width bug (#7940)

GermanJablo · web-flow · commit cd734b0f9826 · 2024-09-11T12:36:54.000-03:00
Closes #7867 Problem: currently, setting an ```ts admin: { width: '30%' } ``` does not work for fields inside a row or similar (group, array etc.) Solution: when we render the field, we set a CSS variable `--field-width` with the value of `admin.width`. This allows us to calculate the correct width for a field in CSS by doing `flex: 0 1 var(--field-width);` It also allows us to properly handle `gap` with `flex-wrap: wrap;` Notes: added playwright tests to ensure widths are correctly rendered ![image](https://github.com/user-attachments/assets/0c0f11fc-2387-4f01-9298-a2613fceee22)
diff --git a/packages/payload/src/fields/config/types.ts b/packages/payload/src/fields/config/types.ts
@@ -264,7 +264,7 @@ type Admin = {
   position?: 'sidebar'
   readOnly?: boolean
   style?: CSSProperties
-  width?: string
+  width?: CSSProperties['width']
 }
 
 export type AdminClient = {
@@ -296,8 +296,8 @@ export type AdminClient = {
   hidden?: boolean
   position?: 'sidebar'
   readOnly?: boolean
-  style?: CSSProperties
-  width?: string
+  style?: { '--field-width'?: CSSProperties['width'] } & CSSProperties
+  width?: CSSProperties['width']
 }
 
 export type Labels = {
@@ -802,7 +802,7 @@ export type UIField = {
      */
     disableListColumn?: boolean
     position?: string
-    width?: string
+    width?: CSSProperties['width']
   }
   /** Extension point to add your custom data. Server only. */
   custom?: Record<string, any>
diff --git a/packages/ui/src/fields/Collapsible/index.tsx b/packages/ui/src/fields/Collapsible/index.tsx
@@ -1,5 +1,5 @@
 'use client'
-import type { CollapsibleFieldClientComponent, DocumentPreferences } from 'payload'
+import type { AdminClient, CollapsibleFieldClientComponent, DocumentPreferences } from 'payload'
 
 import React, { Fragment, useCallback, useEffect, useState } from 'react'
 
@@ -116,6 +116,11 @@ const CollapsibleFieldComponent: CollapsibleFieldClientComponent = (props) => {
 
   const disabled = readOnlyFromProps || readOnlyFromContext || formProcessing || formInitializing
 
+  const style: AdminClient['style'] = {
+    ...field.admin?.style,
+    '--field-width': field.admin.width,
+  }
+
   return (
     <Fragment>
       <WatchChildErrors fields={fields} path={path} setErrorCount={setErrorCount} />
@@ -129,6 +134,7 @@ const CollapsibleFieldComponent: CollapsibleFieldClientComponent = (props) => {
           .filter(Boolean)
           .join(' ')}
         id={`field-${fieldPreferencesKey}${path ? `-${path.replace(/\./g, '__')}` : ''}`}
+        style={style}
       >
         <CollapsibleElement
           className={`${baseClass}__collapsible`}
diff --git a/packages/ui/src/fields/Password/types.ts b/packages/ui/src/fields/Password/types.ts
@@ -9,7 +9,7 @@ import type {
   StaticDescription,
   TextFieldClient,
 } from 'payload'
-import type { ChangeEvent } from 'react'
+import type { CSSProperties, ChangeEvent } from 'react'
 import type React from 'react'
 import type { MarkOptional } from 'ts-essentials'
 
@@ -47,5 +47,5 @@ export type PasswordInputProps = {
   readonly showError?: boolean
   readonly style?: React.CSSProperties
   readonly value?: string
-  readonly width?: string
+  readonly width?: CSSProperties['width']
 }
diff --git a/packages/ui/src/fields/Row/index.scss b/packages/ui/src/fields/Row/index.scss
@@ -4,10 +4,20 @@
   .row__fields {
     display: flex;
     flex-wrap: wrap;
-    gap: calc(var(--base) * 0.5);
+    row-gap: calc(var(--base) * 0.8);
 
     > * {
-      flex-grow: 1;
+      flex: 0 1 var(--field-width);
+    }
+
+    // If there is more than one child, add inline-margins to space them out.
+    &:has(> *:nth-child(2)) {
+      margin-inline: calc(var(--base) / -4); // add negative margin to counteract the gap.
+
+      > * {
+        flex: 0 1 calc(var(--field-width) - var(--base) * 0.5);
+        margin-inline: calc(var(--base) / 4);
+      }
     }
   }
 
diff --git a/packages/ui/src/fields/Select/Input.tsx b/packages/ui/src/fields/Select/Input.tsx
@@ -47,7 +47,7 @@ export type SelectInputProps = {
   readonly showError?: boolean
   readonly style?: React.CSSProperties
   readonly value?: string | string[]
-  readonly width?: string
+  readonly width?: React.CSSProperties['width']
 }
 
 export const SelectInput: React.FC<SelectInputProps> = (props) => {
diff --git a/packages/ui/src/fields/Text/types.ts b/packages/ui/src/fields/Text/types.ts
@@ -41,5 +41,5 @@ export type TextInputProps = {
   readonly style?: React.CSSProperties
   readonly value?: string
   readonly valueToRender?: Option[]
-  readonly width?: string
+  readonly width?: React.CSSProperties['width']
 } & SharedTextFieldProps
diff --git a/packages/ui/src/fields/Textarea/types.ts b/packages/ui/src/fields/Textarea/types.ts
@@ -38,5 +38,5 @@ export type TextAreaInputProps = {
   readonly style?: React.CSSProperties
   readonly value?: string
   readonly valueToRender?: string
-  readonly width?: string
+  readonly width?: React.CSSProperties['width']
 }
diff --git a/packages/ui/src/fields/Upload/Input.tsx b/packages/ui/src/fields/Upload/Input.tsx
@@ -75,7 +75,7 @@ export type UploadInputProps = {
   readonly showError?: boolean
   readonly style?: React.CSSProperties
   readonly value?: (number | string)[] | (number | string)
-  readonly width?: string
+  readonly width?: React.CSSProperties['width']
 }
 
 export function UploadInput(props: UploadInputProps) {
diff --git a/packages/ui/src/providers/Config/createClientConfig/fields.tsx b/packages/ui/src/providers/Config/createClientConfig/fields.tsx
@@ -115,6 +115,22 @@ export const createClientField = ({
 
   const serverProps = { serverProps: { field: incomingField } }
 
+  if ('admin' in incomingField && 'width' in incomingField.admin) {
+    clientField.admin.style = {
+      ...clientField.admin.style,
+      '--field-width': clientField.admin.width,
+      width: undefined, // avoid needlessly adding this to the element's style attribute
+    }
+  } else {
+    if (!(clientField.admin instanceof Object)) {
+      clientField.admin = {}
+    }
+    if (!(clientField.admin.style instanceof Object)) {
+      clientField.admin.style = {}
+    }
+    clientField.admin.style.flex = '1 1 auto'
+  }
+
   switch (incomingField.type) {
     case 'array':
     case 'group':
@@ -521,7 +537,7 @@ export const createClientFields = ({
     })
 
     if (newField) {
-      newClientFields.push({ ...newField })
+      newClientFields.push(newField)
     }
   }
 
diff --git a/test/fields/collections/Row/index.ts b/test/fields/collections/Row/index.ts
@@ -51,7 +51,36 @@ const RowFields: CollectionConfig = {
       type: 'row',
       fields: [
         {
-          label: 'Collapsible within a row',
+          name: 'field_with_width_30_percent',
+          label: 'Field with 30% width',
+          type: 'text',
+          admin: {
+            width: '30%',
+          },
+        },
+        {
+          name: 'field_with_width_60_percent',
+          label: 'Field with 60% width',
+          type: 'text',
+          admin: {
+            width: '60%',
+          },
+        },
+        {
+          name: 'field_with_width_20_percent',
+          label: 'Field with 20% width',
+          type: 'text',
+          admin: {
+            width: '20%',
+          },
+        },
+      ],
+    },
+    {
+      type: 'row',
+      fields: [
+        {
+          label: 'Collapsible 30% width within a row',
           type: 'collapsible',
           fields: [
             {
@@ -60,6 +89,9 @@ const RowFields: CollectionConfig = {
               type: 'text',
             },
           ],
+          admin: {
+            width: '30%',
+          },
         },
         {
           label: 'Collapsible within a row',
@@ -74,6 +106,37 @@ const RowFields: CollectionConfig = {
         },
       ],
     },
+    {
+      type: 'row',
+      fields: [
+        {
+          label: 'Explicit 20% width within a row (A)',
+          type: 'text',
+          name: 'field_20_percent_width_within_row_a',
+          admin: {
+            width: '20%',
+          },
+        },
+        {
+          label: 'No set width within a row (B)',
+          type: 'text',
+          name: 'no_set_width_within_row_b',
+        },
+        {
+          label: 'No set width within a row (C)',
+          type: 'text',
+          name: 'no_set_width_within_row_c',
+        },
+        {
+          label: 'Explicit 20% width within a row (D)',
+          type: 'text',
+          name: 'field_20_percent_width_within_row_d',
+          admin: {
+            width: '20%',
+          },
+        },
+      ],
+    },
   ],
 }
 
diff --git a/test/fields/e2e.spec.ts b/test/fields/e2e.spec.ts
@@ -1,4 +1,4 @@
-import type { Page } from '@playwright/test'
+import type { Locator, Page } from '@playwright/test'
 
 import { expect, test } from '@playwright/test'
 import path from 'path'
@@ -374,20 +374,89 @@ describe('fields', () => {
     test('should render row fields inline and with explicit widths', async () => {
       await page.goto(url.create)
       const fieldA = page.locator('input#field-field_with_width_a')
-      await expect(fieldA).toBeVisible()
       const fieldB = page.locator('input#field-field_with_width_b')
+
+      await expect(fieldA).toBeVisible()
       await expect(fieldB).toBeVisible()
+
       const fieldABox = await fieldA.boundingBox()
       const fieldBBox = await fieldB.boundingBox()
 
-      // Check that the top value of the fields are the same
-      // Give it some wiggle room of like 2px to account for differences in rendering
-      const tolerance = 2
-      expect(fieldABox.y).toBeLessThanOrEqual(fieldBBox.y + tolerance)
+      await expect(() => {
+        expect(fieldABox.y).toEqual(fieldBBox.y)
+        expect(fieldABox.width).toEqual(fieldBBox.width)
+      }).toPass()
 
-      // Check that the widths of the fields are the same
-      const difference = Math.abs(fieldABox.width - fieldBBox.width)
-      expect(difference).toBeLessThanOrEqual(tolerance)
+      const field_30_percent = page.locator(
+        '.field-type.text:has(input#field-field_with_width_30_percent)',
+      )
+      const field_60_percent = page.locator(
+        '.field-type.text:has(input#field-field_with_width_60_percent)',
+      )
+      const field_20_percent = page.locator(
+        '.field-type.text:has(input#field-field_with_width_20_percent)',
+      )
+      const collapsible_30_percent = page.locator(
+        '.collapsible-field:has(#field-field_within_collapsible_a)',
+      )
+
+      const field_20_percent_width_within_row_a = page.locator(
+        '.field-type.text:has(input#field-field_20_percent_width_within_row_a)',
+      )
+      const field_no_set_width_within_row_b = page.locator(
+        '.field-type.text:has(input#field-no_set_width_within_row_b)',
+      )
+      const field_no_set_width_within_row_c = page.locator(
+        '.field-type.text:has(input#field-no_set_width_within_row_c)',
+      )
+      const field_20_percent_width_within_row_d = page.locator(
+        '.field-type.text:has(input#field-field_20_percent_width_within_row_d)',
+      )
+
+      await expect(field_30_percent).toBeVisible()
+      await expect(field_60_percent).toBeVisible()
+      await expect(field_20_percent).toBeVisible()
+      await expect(collapsible_30_percent).toBeVisible()
+      await expect(field_20_percent_width_within_row_a).toBeVisible()
+      await expect(field_no_set_width_within_row_b).toBeVisible()
+      await expect(field_no_set_width_within_row_c).toBeVisible()
+      await expect(field_20_percent_width_within_row_d).toBeVisible()
+
+      const field_30_boundingBox = await field_30_percent.boundingBox()
+      const field_60_boundingBox = await field_60_percent.boundingBox()
+      const field_20_boundingBox = await field_20_percent.boundingBox()
+      const collapsible_30_boundingBox = await collapsible_30_percent.boundingBox()
+      const field_20_percent_width_within_row_a_box =
+        await field_20_percent_width_within_row_a.boundingBox()
+      const field_no_set_width_within_row_b_box =
+        await field_no_set_width_within_row_b.boundingBox()
+      const field_no_set_width_within_row_c_box =
+        await field_no_set_width_within_row_c.boundingBox()
+      const field_20_percent_width_within_row_d_box =
+        await field_20_percent_width_within_row_d.boundingBox()
+
+      await expect(() => {
+        expect(field_30_boundingBox.y).toEqual(field_60_boundingBox.y)
+        expect(field_30_boundingBox.x).toEqual(field_20_boundingBox.x)
+        expect(field_30_boundingBox.y).not.toEqual(field_20_boundingBox.y)
+        expect(field_30_boundingBox.height).toEqual(field_60_boundingBox.height)
+        expect(collapsible_30_boundingBox.width).toEqual(field_30_boundingBox.width)
+
+        expect(field_20_percent_width_within_row_a_box.y).toEqual(
+          field_no_set_width_within_row_b_box.y,
+        )
+        expect(field_no_set_width_within_row_b_box.y).toEqual(field_no_set_width_within_row_c_box.y)
+        expect(field_no_set_width_within_row_c_box.y).toEqual(
+          field_20_percent_width_within_row_d_box.y,
+        )
+
+        expect(field_20_percent_width_within_row_a_box.width).toEqual(
+          field_20_percent_width_within_row_d_box.width,
+        )
+        expect(field_no_set_width_within_row_b_box.width).toEqual(
+          field_no_set_width_within_row_c_box.width,
+        )
+      }).toPass()
     })
 
     test('should render nested row fields in the correct position', async () => {
@@ -402,15 +471,11 @@ describe('fields', () => {
       const fieldABox = await fieldA.boundingBox()
       const fieldBBox = await fieldB.boundingBox()
 
-      // Check that the top value of the fields are the same
-      // Give it some wiggle room of like 2px to account for differences in rendering
-      const tolerance = 2
-      expect(fieldABox.y).toBeLessThanOrEqual(fieldBBox.y + tolerance)
-
-      // Check that the widths of the fields are the same
-      const collapsibleDifference = Math.abs(fieldABox.width - fieldBBox.width)
-
-      expect(collapsibleDifference).toBeLessThanOrEqual(tolerance)
+      await expect(() => {
+        // Check that the top value of the fields are the same
+        expect(fieldABox.y).toEqual(fieldBBox.y)
+        expect(fieldABox.height).toEqual(fieldBBox.height)
+      }).toPass()
     })
   })
 

Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ export type SelectInputProps = {`
`47`	`47`	`readonly showError?: boolean`
`48`	`48`	`readonly style?: React.CSSProperties`
`49`	`49`	`readonly value?: string \| string[]`
`50`		`- readonly width?: string`
	`50`	`+ readonly width?: React.CSSProperties['width']`
`51`	`51`	`}`
`52`	`52`
`53`	`53`	`export const SelectInput: React.FC<SelectInputProps> = (props) => {`
Original file line number	Diff line number	Diff line change
`@@ -38,5 +38,5 @@ export type TextAreaInputProps = {`
`38`	`38`	`readonly style?: React.CSSProperties`
`39`	`39`	`readonly value?: string`
`40`	`40`	`readonly valueToRender?: string`
`41`		`- readonly width?: string`
	`41`	`+ readonly width?: React.CSSProperties['width']`
`42`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -75,7 +75,7 @@ export type UploadInputProps = {`
`75`	`75`	`readonly showError?: boolean`
`76`	`76`	`readonly style?: React.CSSProperties`
`77`	`77`	`readonly value?: (number \| string)[] \| (number \| string)`
`78`		`- readonly width?: string`
	`78`	`+ readonly width?: React.CSSProperties['width']`
`79`	`79`	`}`
`80`	`80`
`81`	`81`	`export function UploadInput(props: UploadInputProps) {`