carbon-design-system / carbon-components-svelte

Svelte implementation of the Carbon Design System
https://svelte.carbondesignsystem.com
Apache License 2.0
2.68k stars 261 forks source link

Support nested properties in DataTable filtering search #1897

Open ChristianRiesen opened 8 months ago

ChristianRiesen commented 8 months ago

Here I have a simple example of what I mean:

<DataTable
  headers={[
    { key: "id", value: "id" },
    { key: "contact.company", value: "Company name" },
  ]}
  {rows}
  {pageSize}
  {page}
>
  <Toolbar>
    <ToolbarContent>
      <ToolbarSearch
        persistent
        value=""
        shouldFilterRows
        bind:filteredRowIds
      />
    </ToolbarContent>
  </Toolbar>
</DataTable>

<Pagination
  bind:pageSize
  bind:page
  totalItems={filteredRowIds.length}
  pageSizeInputDisabled
/>

This example works by showing the correct field in the table. However, when using the search filter it will not find anything in the contact.company field, but it will find a match in the id field.

My guess is the culprit is somewhere here: https://github.com/carbon-design-system/carbon-components-svelte/blob/f1cafd49596e8957ac436a3c7cb7b2645ff86417/src/DataTable/ToolbarSearch.svelte#L57-L67

The table itself I seen in a past issue had a similar problem. https://github.com/carbon-design-system/carbon-components-svelte/pull/602/files

My guess would be the fix now lives around here in the more modern version of the code: https://github.com/carbon-design-system/carbon-components-svelte/blob/f1cafd49596e8957ac436a3c7cb7b2645ff86417/src/DataTable/DataTable.svelte#L199-L206

I'm not familiar enough with all pieces involved to make a proper PR, but it seems like a reasonably straight forward thing to fix from the little I do understand about the code. It would be greatly appriciated.

ChristianRiesen commented 8 months ago

Playing around a bit locally I got some ugly version to work with the following code, which of course is horrible:

function getNestedValue(obj, path) {
  return path.split('.').reduce((acc, part) => acc && acc[part], obj);
}

$: if (shouldFilterRows) {
  let rows = originalRows;
  if (value.trim().length > 0) {
    if (shouldFilterRows === true) {
      rows = rows.filter((row) => {
        return Object.entries(row) // Use entries to get key-value pairs
          .filter(([key]) => key !== "id")
          .some(([key, _value]) => {
            if (typeof _value === "object" && _value !== null) {
              // Handling nested objects
              return Object.entries(_value).some(([nestedKey, nestedValue]) => {
                if (typeof nestedValue === "string" || typeof nestedValue === "number") {
                  return nestedValue
                    .toString()
                    .toLowerCase()
                    .includes(value.trim().toLowerCase());
                }
                return false;
              });
            } else if (typeof _value === "string" || typeof _value === "number") {
              // Handling non-object (string/number) values
              return _value
                .toString()
                .toLowerCase()
                .includes(value.trim().toLowerCase());
            }
            return false;
          });
      });
    } else if (typeof shouldFilterRows === "function") {
      rows = rows.filter((row) => shouldFilterRows(row, value) ?? false);
    }
  }

  tableRows.set(rows);
  filteredRowIds = rows.map((row) => row.id);
}

So it searches through all data available, not just the visible data. Though if I remove the id column from the table, it no longer searches in that. There is some odd behavior here from a usage perspective. My expectation was that it uses the header defined fields, which is not the case.

I will give it a go with a user defined function next in the hopes of not needing my ugly hack. But a generic solution would be pretty welcome I think.

ChristianRiesen commented 8 months ago

The horribleness continues, however this does the job for my usecase for the very moment in time. It's crappy if you have have nested replies, so perhaps having limited or flattened replies would be better for other use cases, or the ability to specify which fields to look for and supporting nested properties would be great.

Just pasting the Toolbar here for brevity, the rest is bog standard.

      <ToolbarSearch
        persistent
        value=""
        shouldFilterRows={(row, value) => {
          const searchValue = value.trim().toLowerCase();

          function searchInObject(obj) {
            return Object.entries(obj).some(([key, val]) => {
              if (typeof val === 'object' && val !== null) {
                return searchInObject(val);
              } else {
                return val.toString().toLowerCase().includes(searchValue);
              }
            });
          }

          // Check if the value is in the id or anywhere else in the row
          return searchInObject(row) || row.id.toString().toLowerCase().includes(searchValue);
        }}
        bind:filteredRowIds
      />
theetrain commented 8 months ago

I was able to reproduce the issue; would be a good fix for DataTable.

Reproduction: https://svelte.dev/repl/4012e79a919f42d6b3fe5179ef774757?version=4.2.9 (cannot search for 'super').