carbon-design-system / carbon-components-svelte

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

feat(data-table): pass `row` to `display` function #1810

Closed Pierstoval closed 1 year ago

Pierstoval commented 1 year ago

This will allow display usage to use the whole object for use with computed values, or when needing multiple fields to show more than one element (like "item actions" that might need more than one field, for instance).

Pierstoval commented 1 year ago

Can you update the JSDoc types in DataTable.svelte (Lines 5, 6, 10) and run yarn build:docs?

According to the docs, only line 10 needs update: the "display" method receives the row object only for cells, not headers, am I right?

Just rebased & pushed an update

metonym commented 1 year ago

I believe that the display is reflected in the DataTableNonEmptyHeader and DataTableEmptyHeader interfaces.

When testing the types, I see this:

Screenshot 2023-09-28 at 8 53 59 AM

Repro: try using row in the type test for DataTable: https://github.com/carbon-design-system/carbon-components-svelte/blob/aae2dd42aa6b9b69fe7864564d23e8a823f6e340/tests/DataTable.test.svelte#L179

Pierstoval commented 1 year ago

I don't see a reason to make this change on headers: row object should not be accessible when displaying headers 🤔

This means two things:

I think we should change the behavior for that.

I just pushed a fix to add row?: ... to header-related display functions, and kept row: ... for cell-related.

I'm not sure the rest of the PR is okay, feel free to make more comments 👌

Pierstoval commented 1 year ago

Thank you @metonym !

metonym commented 1 year ago

Released in v0.81.0