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

Select event table #2822

Open
wants to merge 6 commits into
base: v3
Choose a base branch
from
Open

Select event table #2822

wants to merge 6 commits into from

Conversation

clopezpro
Copy link
Contributor

@clopezpro clopezpro commented Dec 2, 2024

πŸ”— Linked issue

❓ Type of change

  • [X ] πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality)
  • [X ] ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

In version 2 you could use an @select event which could be used to select the table manually or open some modal, I don't know if this can suddenly be achieved in another way with tanstack

πŸ“ Checklist

  • I have linked an issue or discussion.
  • [ x] I have updated the documentation accordingly.

Copy link
Member

@benjamincanac benjamincanac left a comment

Choose a reason for hiding this comment

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

@clopezpro clopezpro mentioned this pull request Dec 16, 2024
11 tasks
@@ -229,7 +238,7 @@ defineExpose({
<tbody :class="ui.tbody({ class: [props.ui?.tbody] })">
<template v-if="tableApi.getRowModel().rows?.length">
<template v-for="row in tableApi.getRowModel().rows" :key="row.id">
<tr :data-selected="row.getIsSelected()" :data-expanded="row.getIsExpanded()" :class="ui.tr({ class: [props.ui?.tr] })">
<tr :data-selected="row.getIsSelected()" :data-can-select="!!props.onSelect" :data-expanded="row.getIsExpanded()" :class="ui.tr({ class: [props.ui?.tr] })" @click="handleRowSelect(row, $event)">
Copy link
Member

Choose a reason for hiding this comment

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

nice! but out of curiosity, how would this work for accessibility / screenreaders as this behaves like a button?

Copy link
Member

Choose a reason for hiding this comment

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

maybe the tanstack table docs also would be missing to address this (or my question is silly!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe this could be solved if we add this attribute?

:role="props.onSelect ? 'button' : undefined"

Copy link
Member

@ineshbose ineshbose Dec 16, 2024

Choose a reason for hiding this comment

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

It might need tabIndex="0" too (and in that case, should it be white outline, or primary colour, or maybe hover effect that bg changes, maybe for simplity we stick to white outline, similar to anchor tags)
(and implement navigation using arrow keys I suppose)

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.

3 participants