fix(ui): properly filters fields from list view columns and conditions (

#10246) Fixes #10234. Some fields, such as focal point fields for upload enabled collections, were rendering in the condition selector despite being hidden from the column selector. This was because the logic for the column selector was filtering fields without labels, but the same was not being done for the filter conditions. This, however, is not a good way to filter these fields as it requires this specific logic to be written in multiple places. Instead, they need to explicitly check for `hidden` and `disabled` in addition to `disableListFilter` and `disableListColumn`. The actual filtering logic has been improved across the two instances as well, removing multiple duplicative loops. This change has also exposed a underlying issue with the way columns were handled within the table columns provider. When row selections were enabled, the selector columns were present in column state. This caused problems when interacting with column indices, such as when reordering columns. Instead of needing to manually filter these out every time we need to work with column state, they no longer appear there in the first place. Instead, we inject the row selectors directly into the table itself, completely isolating these row selectors from the column state.
payloadcms · Dec 30, 2024 · f5a955d · f5a955d
1 parent 885e966
commit f5a955d
Show file tree

Hide file tree

Showing 13 changed files with 164 additions and 86 deletions.
diff --git a/packages/payload/src/uploads/getBaseFields.ts b/packages/payload/src/uploads/getBaseFields.ts
@@ -170,6 +170,8 @@ export const getBaseUploadFields = ({ collection, config }: Options): Field[] =>
           name,
           type: 'number',
           admin: {
+            disableListColumn: true,
+            disableListFilter: true,
             hidden: true,
           },
         }

diff --git a/packages/ui/src/elements/ColumnSelector/index.tsx b/packages/ui/src/elements/ColumnSelector/index.tsx
@@ -1,9 +1,8 @@
 'use client'
 import type { SanitizedCollectionConfig } from 'payload'
 
-import React, { useId } from 'react'
-
-import type { Column } from '../Table/index.js'
+import { fieldIsHiddenOrDisabled, fieldIsID } from 'payload/shared'
+import React, { useId, useMemo } from 'react'
 
 import { FieldLabel } from '../../fields/FieldLabel/index.js'
 import { PlusIcon } from '../../icons/Plus/index.js'
@@ -20,24 +19,26 @@ export type Props = {
   readonly collectionSlug: SanitizedCollectionConfig['slug']
 }
 
-const filterColumnFields = (columns: Column[]): Column[] => {
-  return columns.filter((c) => {
-    return !c?.field?.admin?.disableListColumn
-  })
-}
-
 export const ColumnSelector: React.FC<Props> = ({ collectionSlug }) => {
   const { columns, moveColumn, toggleColumn } = useTableColumns()
 
   const uuid = useId()
   const editDepth = useEditDepth()
 
+  const filteredColumns = useMemo(
+    () =>
+      columns.filter(
+        (col) =>
+          !(fieldIsHiddenOrDisabled(col.field) && !fieldIsID(col.field)) &&
+          !col?.field?.admin?.disableListColumn,
+      ),
+    [columns],
+  )
+
   if (!columns) {
     return null
   }
 
-  const filteredColumns = filterColumnFields(columns)
-
   return (
     <DraggableSortable
       className={baseClass}
@@ -50,21 +51,8 @@ export const ColumnSelector: React.FC<Props> = ({ collectionSlug }) => {
       }}
     >
       {filteredColumns.map((col, i) => {
-        if (!col) {
-          return null
-        }
-
         const { accessor, active, field } = col
 
-        if (
-          col.accessor === '_select' ||
-          !field ||
-          col.CustomLabel === null ||
-          (col.CustomLabel === undefined && !('label' in field))
-        ) {
-          return null
-        }
-
         return (
           <Pill
             alignIcon="left"
@@ -80,7 +68,9 @@ export const ColumnSelector: React.FC<Props> = ({ collectionSlug }) => {
               void toggleColumn(accessor)
             }}
           >
-            {col.CustomLabel ?? <FieldLabel label={'label' in field && field.label} unstyled />}
+            {col.CustomLabel ?? (
+              <FieldLabel label={field && 'label' in field && field.label} unstyled />
+            )}
           </Pill>
         )
       })}

diff --git a/packages/ui/src/elements/TableColumns/buildColumnState.tsx b/packages/ui/src/elements/TableColumns/buildColumnState.tsx
@@ -279,16 +279,6 @@ export const buildColumnState = (args: Args): Column[] => {
     return acc
   }, [])
 
-  if (enableRowSelections) {
-    sorted?.unshift({
-      accessor: '_select',
-      active: true,
-      field: null,
-      Heading: <SelectAll />,
-      renderedCells: docs.map((_, i) => <SelectRow key={i} rowData={docs[i]} />),
-    })
-  }
-
   if (beforeRows) {
     sorted.unshift(...beforeRows)
   }

diff --git a/packages/ui/src/elements/WhereBuilder/Condition/index.tsx b/packages/ui/src/elements/WhereBuilder/Condition/index.tsx
@@ -1,5 +1,5 @@
 'use client'
-import React, { useEffect, useMemo, useState } from 'react'
+import React, { useEffect, useState } from 'react'
 
 import type { FieldCondition } from '../types.js'
 
@@ -17,9 +17,9 @@ export type Props = {
   }) => void
   readonly andIndex: number
   readonly fieldName: string
-  readonly fields: FieldCondition[]
   readonly initialValue: string
   readonly operator: Operator
+  readonly options: FieldCondition[]
   readonly orIndex: number
   readonly removeCondition: ({ andIndex, orIndex }: { andIndex: number; orIndex: number }) => void
   readonly RenderedFilter: React.ReactNode
@@ -56,21 +56,17 @@ export const Condition: React.FC<Props> = (props) => {
     addCondition,
     andIndex,
     fieldName,
-    fields,
     initialValue,
     operator,
+    options,
     orIndex,
     removeCondition,
     RenderedFilter,
     updateCondition,
   } = props
 
-  const options = useMemo(() => {
-    return fields.filter(({ field }) => !field.admin.disableListFilter)
-  }, [fields])
-
-  const [internalField, setInternalField] = useState<FieldCondition>(() =>
-    fields.find((field) => fieldName === field.value),
+  const [fieldOption, setFieldOption] = useState<FieldCondition>(() =>
+    options.find((field) => fieldName === field.value),
   )
 
   const { t } = useTranslation()
@@ -82,13 +78,13 @@ export const Condition: React.FC<Props> = (props) => {
   useEffect(() => {
     // This is to trigger changes when the debounced value changes
     if (
-      (internalField?.value || typeof internalField?.value === 'number') &&
+      (fieldOption?.value || typeof fieldOption?.value === 'number') &&
       internalOperatorOption &&
       ![null, undefined].includes(debouncedValue)
     ) {
       updateCondition({
         andIndex,
-        fieldName: internalField.value,
+        fieldName: fieldOption.value,
         operator: internalOperatorOption,
         orIndex,
         value: debouncedValue,
@@ -97,15 +93,15 @@ export const Condition: React.FC<Props> = (props) => {
   }, [
     debouncedValue,
     andIndex,
-    internalField?.value,
+    fieldOption?.value,
     internalOperatorOption,
     orIndex,
     updateCondition,
     operator,
   ])
 
   const booleanSelect =
-    ['exists'].includes(internalOperatorOption) || internalField?.field?.type === 'checkbox'
+    ['exists'].includes(internalOperatorOption) || fieldOption?.field?.type === 'checkbox'
 
   let valueOptions
 
@@ -114,13 +110,13 @@ export const Condition: React.FC<Props> = (props) => {
       { label: t('general:true'), value: 'true' },
       { label: t('general:false'), value: 'false' },
     ]
-  } else if (internalField?.field && 'options' in internalField.field) {
-    valueOptions = internalField.field.options
+  } else if (fieldOption?.field && 'options' in fieldOption.field) {
+    valueOptions = fieldOption.field.options
   }
 
   const disabled =
-    (!internalField?.value && typeof internalField?.value !== 'number') ||
-    internalField?.field?.admin?.disableListFilter
+    (!fieldOption?.value && typeof fieldOption?.value !== 'number') ||
+    fieldOption?.field?.admin?.disableListFilter
 
   return (
     <div className={baseClass}>
@@ -131,14 +127,14 @@ export const Condition: React.FC<Props> = (props) => {
               disabled={disabled}
               isClearable={false}
               onChange={(field: Option) => {
-                setInternalField(fields.find((f) => f.value === field.value))
+                setFieldOption(options.find((f) => f.value === field.value))
                 setInternalOperatorOption(undefined)
                 setInternalQueryValue(undefined)
               }}
-              options={options}
+              options={options.filter((field) => !field.field.admin.disableListFilter)}
               value={
-                fields.find((field) => internalField?.value === field.value) || {
-                  value: internalField?.value,
+                options.find((field) => fieldOption?.value === field.value) || {
+                  value: fieldOption?.value,
                 }
               }
             />
@@ -150,9 +146,9 @@ export const Condition: React.FC<Props> = (props) => {
               onChange={(operator: Option<Operator>) => {
                 setInternalOperatorOption(operator.value)
               }}
-              options={internalField?.operators}
+              options={fieldOption?.operators}
               value={
-                internalField?.operators.find(
+                fieldOption?.operators.find(
                   (operator) => internalOperatorOption === operator.value,
                 ) || null
               }
@@ -162,8 +158,12 @@ export const Condition: React.FC<Props> = (props) => {
             {RenderedFilter || (
               <DefaultFilter
                 booleanSelect={booleanSelect}
-                disabled={!internalOperatorOption || internalField?.field?.admin?.disableListFilter}
-                internalField={internalField}
+                disabled={
+                  !internalOperatorOption ||
+                  !fieldOption ||
+                  fieldOption?.field?.admin?.disableListFilter
+                }
+                internalField={fieldOption}
                 onChange={setInternalQueryValue}
                 operator={internalOperatorOption}
                 options={valueOptions}
@@ -194,7 +194,7 @@ export const Condition: React.FC<Props> = (props) => {
             onClick={() =>
               addCondition({
                 andIndex: andIndex + 1,
-                fieldName: fields.find((field) => !field.field.admin?.disableListFilter).value,
+                fieldName: options.find((field) => !field.field.admin?.disableListFilter).value,
                 orIndex,
                 relation: 'and',
               })

diff --git a/packages/ui/src/elements/WhereBuilder/index.tsx b/packages/ui/src/elements/WhereBuilder/index.tsx
@@ -10,10 +10,10 @@ import { useListQuery } from '../../providers/ListQuery/index.js'
 import { useTranslation } from '../../providers/Translation/index.js'
 import { Button } from '../Button/index.js'
 import { Condition } from './Condition/index.js'
-import './index.scss'
 import { reduceClientFields } from './reduceClientFields.js'
 import { transformWhereQuery } from './transformWhereQuery.js'
 import validateWhereQuery from './validateWhereQuery.js'
+import './index.scss'
 
 const baseClass = 'where-builder'
 
@@ -27,10 +27,10 @@ export const WhereBuilder: React.FC<WhereBuilderProps> = (props) => {
   const { collectionPluralLabel, fields, renderedFilters } = props
   const { i18n, t } = useTranslation()
 
-  const [reducedFields, setReducedColumns] = useState(() => reduceClientFields({ fields, i18n }))
+  const [options, setOptions] = useState(() => reduceClientFields({ fields, i18n }))
 
   useEffect(() => {
-    setReducedColumns(reduceClientFields({ fields, i18n }))
+    setOptions(reduceClientFields({ fields, i18n }))
   }, [fields, i18n])
 
   const { handleWhereChange, query } = useListQuery()
@@ -185,9 +185,9 @@ export const WhereBuilder: React.FC<WhereBuilderProps> = (props) => {
                               addCondition={addCondition}
                               andIndex={andIndex}
                               fieldName={initialFieldName}
-                              fields={reducedFields}
                               initialValue={initialValue}
                               operator={initialOperator}
+                              options={options}
                               orIndex={orIndex}
                               removeCondition={removeCondition}
                               RenderedFilter={renderedFilters?.get(initialFieldName)}
@@ -210,7 +210,7 @@ export const WhereBuilder: React.FC<WhereBuilderProps> = (props) => {
             onClick={() => {
               addCondition({
                 andIndex: 0,
-                fieldName: reducedFields[0].value,
+                fieldName: options[0].value,
                 orIndex: conditions.length,
                 relation: 'or',
               })
@@ -230,11 +230,10 @@ export const WhereBuilder: React.FC<WhereBuilderProps> = (props) => {
             iconPosition="left"
             iconStyle="with-border"
             onClick={() => {
-              if (reducedFields.length > 0) {
+              if (options.length > 0) {
                 addCondition({
                   andIndex: 0,
-                  fieldName: reducedFields.find((field) => !field.field.admin?.disableListFilter)
-                    .value,
+                  fieldName: options.find((field) => !field.field.admin?.disableListFilter).value,
                   orIndex: conditions.length,
                   relation: 'or',
                 })

diff --git a/packages/ui/src/elements/WhereBuilder/reduceClientFields.tsx b/packages/ui/src/elements/WhereBuilder/reduceClientFields.tsx
@@ -29,6 +29,7 @@ export const reduceClientFields = ({
   pathPrefix,
 }: ReduceClientFieldsArgs): FieldCondition[] => {
   return fields.reduce((reduced, field) => {
+    // Do not filter out `field.admin.disableListFilter` fields here, as these should still render as disabled if they appear in the URL query
     if (fieldIsHiddenOrDisabled(field) && !fieldIsID(field)) {
       return reduced
     }

diff --git a/packages/ui/src/utilities/renderTable.tsx b/packages/ui/src/utilities/renderTable.tsx
@@ -19,7 +19,7 @@ import { buildColumnState } from '../elements/TableColumns/buildColumnState.js'
 import { filterFields } from '../elements/TableColumns/filterFields.js'
 import { getInitialColumns } from '../elements/TableColumns/getInitialColumns.js'
 // eslint-disable-next-line payload/no-imports-from-exports-dir
-import { Pill, Table } from '../exports/client/index.js'
+import { Pill, SelectAll, SelectRow, Table } from '../exports/client/index.js'
 
 export const renderFilters = (
   fields: Field[],
@@ -91,19 +91,6 @@ export const renderTable = ({
       )
 
   const columnState = buildColumnState({
-    beforeRows: renderRowTypes
-      ? [
-          {
-            accessor: 'collection',
-            active: true,
-            field: null,
-            Heading: i18n.t('version:type'),
-            renderedCells: docs.map((_, i) => (
-              <Pill key={i}>{getTranslation(clientCollectionConfig.labels.singular, i18n)}</Pill>
-            )),
-          },
-        ]
-      : undefined,
     clientCollectionConfig,
     collectionConfig,
     columnPreferences,
@@ -117,8 +104,42 @@ export const renderTable = ({
     useAsTitle,
   })
 
+  const columnsToUse = [...columnState]
+
+  if (renderRowTypes) {
+    columnsToUse.unshift({
+      accessor: 'collection',
+      active: true,
+      field: {
+        admin: {
+          disabled: true,
+        },
+        hidden: true,
+      },
+      Heading: i18n.t('version:type'),
+      renderedCells: docs.map((_, i) => (
+        <Pill key={i}>{getTranslation(clientCollectionConfig.labels.singular, i18n)}</Pill>
+      )),
+    } as Column)
+  }
+
+  if (enableRowSelections) {
+    columnsToUse.unshift({
+      accessor: '_select',
+      active: true,
+      field: {
+        admin: {
+          disabled: true,
+        },
+        hidden: true,
+      },
+      Heading: <SelectAll />,
+      renderedCells: docs.map((_, i) => <SelectRow key={i} rowData={docs[i]} />),
+    } as Column)
+  }
+
   return {
     columnState,
-    Table: <Table appearance={tableAppearance} columns={columnState} data={docs} />,
+    Table: <Table appearance={tableAppearance} columns={columnsToUse} data={docs} />,
   }
 }