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

<DynamicTable /> shades rows on hover when clickable #62

Closed
wants to merge 2 commits into from

Conversation

DavidLarsKetch
Copy link
Collaborator

This PR adds DynamicTableOptions.clickable. When clickable and the cursor hovers over a row, additional styling applies to help differentiate the fact that is the target of a click.

Screen.Recording.2024-04-19.at.8.41.11.AM.mov

@steven-klein
Copy link
Collaborator

I'm conflicted on this a bit, since we are down to a single instance of @click:row on the notifications table in portal, and I'm not sure why we held on to it there. Looking at the groups table usage in #1223 it feels like an action button of "View" or similar would be ok especially at this juncture in admin tooling. If nothing else, we continue to steer ourselves towards consistent usage of tables until we get some user feedback that pushes us in another direction.

I do think this highlights the potential need for "selectable" rows, especially in admin scenarios. Either single select or multiselect (which Archive has an implementation of) where "selecting" rows isn't duplicating the functionality of ActionItems i.e. firing a callback, but is populating a v-model:selected with the selected item or items. That theoretically opens up some new functionalities, like selecting a row to display additional details or edit forms in an adjacent component. I'm definitely speculating, but separating "selections" and "actions" probably has the most long term value for us.

Happy to PL. Otherwise, toss in a "View" button to keep moving forward.

@DavidLarsKetch
Copy link
Collaborator Author

Definitely a good PL and not sure I'm on board with moving away from clickability of rows. In the case of #1223, I don't want action items to expand beyond the single use of the table, which is navigation. Something I'll think about over the weekend, of course.

One point I'd make separate from the action conversation is just the need for some contrast between rows on tables. Using the <DetailList /> example in dev/, it feels natural to throw that hover:bg-gray-50 so the user has a way to focus in on a row in the table (or an item in the list). Dropping cursor-pointer seems like a fair compromise for now, in my opinion, as our users will get used to the fact that a table's rows are clickable, generally, and some tables do something with that, others don't. The cursor isn't there to confirm/deny it's clickable.

@DavidLarsKetch DavidLarsKetch deleted the davidlarsketch/dynamictable-adds branch April 22, 2024 16:35
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