-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[UI v2] Variables table - part 2 #16025
Conversation
<div> | ||
<div className="grid sm:grid-cols-2 md:grid-cols-6 lg:grid-cols-12 gap-2 pb-4 items-center"> | ||
<div className="sm:col-span-2 md:col-span-6 lg:col-span-4 order-last lg:order-first"> | ||
<p className="text-sm text-muted-foreground"> | ||
{currentVariableCount} Variables | ||
</p> | ||
</div> |
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.
I see the CREATE action is in the parent component, but the filtering fields are colocated with the table itself. This is totally fine, but how are you thinking about that in general?
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.
The filters feel more related to the table than the parent component since I can't imagine a scenario where you'd want the table without the filters, but I'm open to changing this layout as we build more.
}) => { | ||
columnFilters, | ||
onColumnFiltersChange, | ||
searchDebounceMs = 500, |
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.
let's keep this south of ~200-250ms
This PR finishes up the variables table by adding filtering, page size selection, and sorting to the table. This PR also refactors the
TagsInput
component slightly to fit into the table filtering layout a little better.Here are some of the changes in action:
Related to #15512