-
Notifications
You must be signed in to change notification settings - Fork 278
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
feat: implement sorting and searching in Table component #1425
Conversation
@aarondoet is attempting to deploy a commit to the Themesberg Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes introduce enhancements to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Table
participant TableBody
participant TableHeadCell
User->>Table: Input search term
Table->>TableBody: Update searchTerm
TableBody->>TableBody: Filter items based on searchTerm
TableBody->>TableHeadCell: Request sorting function
TableHeadCell->>TableBody: Provide sorting logic
TableBody->>User: Display filtered and sorted items
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
src/routes/component-data/Table.json (1)
1-1
: LGTM! The new props enhance the component's functionality and customization options.The expansion of the
props
array in theTable
component's configuration introduces several valuable improvements:
- The
"items"
prop of typeT[]
enables the component to handle generic data, enhancing its type safety and reusability across different data types.- The
"filter"
prop allows for custom filtering logic to be passed to the component, providing flexibility in how the data is filtered based on the search term.- The
"placeholder"
prop enables customization of the search input's placeholder text, improving the user experience.- The various class props offer fine-grained control over the styling of different elements within the component, allowing for better visual customization.
The default values provided for the props are sensible and maintain a consistent pattern.
To further improve the configuration, consider adding JSDoc comments to document the purpose and expected values for each prop. This will enhance the developer experience and make it easier for others to understand and use the component correctly.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- src/lib/table/Table.svelte (4 hunks)
- src/lib/table/TableBody.svelte (1 hunks)
- src/lib/table/TableHeadCell.svelte (1 hunks)
- src/routes/component-data/Table.json (1 hunks)
- src/routes/component-data/TableBody.json (1 hunks)
- src/routes/component-data/TableHeadCell.json (1 hunks)
- src/routes/docs/components/table.md (2 hunks)
Additional comments not posted (23)
src/routes/component-data/TableBody.json (1)
1-1
: LGTM! The addition of the "row" slot enhances the TableBody component.The code change is approved. The addition of the
"row"
slot to theTableBody
component definition is a valid enhancement that allows for the inclusion of row elements within the component. This change expands the component's capabilities by enabling the rendering of rows dynamically while preserving its existing structure and properties, ensuring backward compatibility.src/routes/component-data/TableHeadCell.json (1)
1-1
: LGTM! But consider adding a type constraint and verify the usage of thesort
property.The new properties
sort
,defaultDirection
,defaultSort
, anddirection
enhance the component's capabilities for handling sorting behavior, making it more versatile in data presentation contexts. The changes are well-structured and provide clear default values.However, please consider adding a type constraint to the generic type
T
to ensure it's comparable. This will help catch type errors at compile time.Apply this diff to add the type constraint:
-["sort","((a: T, b: T) => number) | null = null;",""] +["sort","((a: T extends Comparable, b: T extends Comparable) => number) | null = null;",""]Also, ensure that the sorting function passed to the
sort
property is properly typed and handles the generic typeT
correctly.Run the following script to verify the usage of the
sort
property:src/routes/component-data/Table.json (1)
1-1
: LGTM! The new slots enhance the component's extensibility.The addition of the
"search"
,"svgSearch"
, and"header"
slots to theTable
component's configuration is a positive change that improves the component's extensibility and flexibility. The naming of the slots is clear and follows a consistent pattern, making it easy for developers to understand their purpose.src/lib/table/TableBody.svelte (4)
1-1
: LGTM!The addition of the
generics
attribute with a type parameterT
is a great way to make the component more flexible and reusable by allowing it to work with various data types.
7-11
: LGTM!The enhanced context retrieval and reactive statements for filtering and sorting
items
are well-implemented and improve the component's capability to manage and display data effectively. The checks for defined filter and sorter functions ensure proper handling of optional functionalities.
17-19
: LGTM!Iterating over the
sorted
array for rendering table rows is a great way to dynamically generate rows based on the sorted data. This change enhances the component's capability to display data more effectively, taking into account the applied sorting criteria.
Line range hint
23-27
: LGTM!The documentation comment is helpful for understanding the component's usage and available props. It is up-to-date with the current implementation.
src/lib/table/TableHeadCell.svelte (5)
1-3
: LGTM!The code changes are approved. The use of generics and imports are correctly implemented.
7-10
: LGTM!The code changes are approved. The exported properties for sorting functionality are correctly implemented.
12-14
: LGTM!The code changes are approved. The retrieval of the
sorter
context and the reactive update of thedirection
are correctly implemented.
16-29
: LGTM!The code changes are approved. The
sortItems
function and the logic for updating thesorter
context are correctly implemented.
32-42
: LGTM!The code changes are approved. The component's rendering logic and the conditional display of the sorting button are correctly implemented.
src/lib/table/Table.svelte (6)
1-1
: LGTM!The introduction of the generic type parameter
T
is a good practice for making the component more reusable.
14-15
: LGTM!The new properties
items
andfilter
are correctly typed using the generic type parameterT
. This allows for passing data to the table component and enables dynamic filtering of the items based on user input.
16-21
: LGTM!The new properties for customizing the search input and its associated elements provide flexibility and follow the project's coding style.
23-25
: LGTM!The
searchTerm
variable is necessary for managing the state of the search input. UsingtwMerge
for merging Tailwind CSS classes is a good practice.
43-50
: LGTM!Sharing
items
,searchTerm
,filter
, andsorter
using the context API enhances the component's interactivity and responsiveness to user input. Using writable stores forsearchTerm
,filter
, andsorter
facilitates reactivity in the component.
54-71
: LGTM!Conditional rendering of the search input and its associated elements using Svelte's
{#if}
block ensures that the search functionality is only displayed if a filter function is provided. This improves performance by not rendering unnecessary elements.src/routes/docs/components/table.md (5)
255-264
: LGTM!The code changes are approved.
272-277
: LGTM!The code changes are approved. Using the
slot="row"
andlet:item
attributes inTableBodyRow
allows for flexible rendering of table rows based on theitems
data.
297-303
: LGTM!The code changes are approved. The
sort
prop allows specifying custom sorting logic for each column. ThedefaultSort
anddefaultDirection
props enable sorting by default on specific columns.
306-314
: LGTM!The code changes are approved. Using the
slot="row"
andlet:item
attributes inTableBodyRow
allows for flexible rendering of table rows based on theitems
data. The anchor tag in the lastTableBodyCell
provides an action to buy the item.
915-915
: LGTM!The code changes are approved. The
GitHubCompoLinks
component is likely used to display links to the component's GitHub repository.
{"name":"TableHeadCell","slots":[],"events":["on:click","on:focus","on:keydown","on:keypress","on:keyup","on:mouseenter","on:mouseleave","on:mouseover","on:click","on:focus","on:keydown","on:keypress","on:keyup","on:mouseenter","on:mouseleave","on:mouseover"],"props":[["padding","string","'px-6 py-3'"],["sort","((a: T, b: T) => number) | null = null;",""],["defaultDirection","'asc' | 'desc'","'asc'"],["defaultSort","boolean","false"],["direction","'asc' | 'desc' | null","defaultSort ? defaultDirection : null"]]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the duplicated events in the events
array.
The events
array contains the same set of events twice:
"events":["on:click","on:focus","on:keydown","on:keypress","on:keyup","on:mouseenter","on:mouseleave","on:mouseover","on:click","on:focus","on:keydown","on:keypress","on:keyup","on:mouseenter","on:mouseleave","on:mouseover"]
This duplication is redundant and can lead to confusion. Please remove one set of the duplicated events.
Apply this diff to remove the duplicated events:
-"events":["on:click","on:focus","on:keydown","on:keypress","on:keyup","on:mouseenter","on:mouseleave","on:mouseover","on:click","on:focus","on:keydown","on:keypress","on:keyup","on:mouseenter","on:mouseleave","on:mouseover"]
+"events":["on:click","on:focus","on:keydown","on:keypress","on:keyup","on:mouseenter","on:mouseleave","on:mouseover"]
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
As far as I remember, you don't need |
I do get errors that T is not defined when removing that. |
Thank you for the PR. |
Replaces #1423
📑 Description
In #1423 I gave it a try to implement sorting within the Table component. I thought that searching would be fairly easy with that setup, when the table already has the raw data itself. So I tried to make a table that can easily be both searched and sorted, this is my current attempt.
Status
✅ Checks
Additional Information
I would deprecate the TableSearch component with this but don't know if/how that is possible. I don't want to completely delete it yet for backwards compatibility.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation