Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[sparkle] - feature: enhance DataTable with interactive row elements #6617

Merged
merged 5 commits into from
Aug 1, 2024

Conversation

JulesBelveze
Copy link
Contributor

Description

This PR aims at enhancing the DataTable component in sparkle. Main changes include:

  • Add functionality to DataTable rows for additional interactivity, enabling click events on the entire row
  • Introduce a 'showMore' option with its corresponding 'onMoreClick' callback to handle secondary actions within rows

Risk

None

Deploy Plan

  • Deploy sparkle

 - Add functionality to DataTable rows for additional interactivity, enabling click events on the entire row
 - Introduce a 'showMore' option with its corresponding 'onMoreClick' callback to handle secondary actions within rows
@JulesBelveze JulesBelveze self-assigned this Aug 1, 2024
@JulesBelveze JulesBelveze requested a review from flvndvd August 1, 2024 09:55
 - Update @dust-tt/sparkle package version for release
@@ -58,7 +58,7 @@ export function DataTable<TData, TValue>({
<DataTable.Root className={className}>
<DataTable.Header>
{table.getHeaderGroups().map((headerGroup) => (
<DataTable.Row key={headerGroup.id}>
<DataTable.Row key={headerGroup.id} >
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<DataTable.Row key={headerGroup.id} >
<DataTable.Row key={headerGroup.id}>

Comment on lines 102 to 103
showMore={row.original.showMore}
onMoreClick={row.original.onMoreClick}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we infer the showMore from onMoreClick existence?

Jules added 3 commits August 1, 2024 13:24
 - Removed `clickable` and `showMore` boolean flags to simplify Row component's API and usage
 - Adjusted conditional class and event handling logic to depend on the presence of event handlers directly, enhancing readability and maintainability of the code
…ponent

 - Removed unnecessary `clickable` and `showMore` props from DataTable.Row to simplify the component interface
 - Retained `onClick` and `onMoreClick` handlers for row interactions
 - Change the margin and width of the sorting icons for better alignment within table headers
@JulesBelveze JulesBelveze merged commit 2812b98 into main Aug 1, 2024
3 checks passed
@JulesBelveze JulesBelveze deleted the enh/sparkle/datatable branch August 1, 2024 11:34
albandum pushed a commit that referenced this pull request Aug 28, 2024
…6617)

* [sparkle] - feature: enhance DataTable with interactive row elements

 - Add functionality to DataTable rows for additional interactivity, enabling click events on the entire row
 - Introduce a 'showMore' option with its corresponding 'onMoreClick' callback to handle secondary actions within rows

* [sparkle] - feature: bump package version to 0.2.198

 - Update @dust-tt/sparkle package version for release

* [sparkle] - refactor: streamline DataTable Row props and behavior

 - Removed `clickable` and `showMore` boolean flags to simplify Row component's API and usage
 - Adjusted conditional class and event handling logic to depend on the presence of event handlers directly, enhancing readability and maintainability of the code

* [sparkle] - refactor: streamline DataTable.Row props in DataTable component

 - Removed unnecessary `clickable` and `showMore` props from DataTable.Row to simplify the component interface
 - Retained `onClick` and `onMoreClick` handlers for row interactions

* [sparkle] - fix: adjust sorting icon position in DataTable

 - Change the margin and width of the sorting icons for better alignment within table headers

---------

Co-authored-by: Jules <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants