Skip to content

feat(table): Add prop isRowSelectable - #2228

Draft
anandBalachandar wants to merge 4 commits into
finos:mainfrom
anandBalachandar:feature/table-checkbox-rowlevel-props
Draft

feat(table): Add prop isRowSelectable#2228
anandBalachandar wants to merge 4 commits into
finos:mainfrom
anandBalachandar:feature/table-checkbox-rowlevel-props

Conversation

@anandBalachandar

@anandBalachandar anandBalachandar commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

feat(table): Add prop isRowSelectable

  • isRowSelectable: (dataRow: DataRow) => boolean on TableProps.
  • Non-selectable rows are skipped in useSelection (all selection models — checkbox, extended, single, block).
  • They receive a vuuTableRow-noSelect CSS class which styles the checkbox as visually disabled.
  • The checkbox checked state is preserved for non-selectable rows even when isSelected is cleared by a subsequent selection.
  • Add Playwright CT test for soft-delete checkbox persistence across row selection
  • Document in README

…state

- Introduce CheckboxRowLevelProps
- Apply in EditableInstrumentsTemplate: soft-deleted rows show checked+disabled
  checkbox via { disabled: true, checked: true }
- Add Playwright CT test for soft-delete checkbox persistence across row selection
- Document in README
@netlify

netlify Bot commented Jul 23, 2026

Copy link
Copy Markdown

Deploy Preview for papaya-valkyrie-395400 canceled.

Name Link
🔨 Latest commit 29f8e04
🔍 Latest deploy log https://app.netlify.com/projects/papaya-valkyrie-395400/deploys/6a66522e0b56c000086d325b

Comment thread vuu-ui/packages/vuu-table/src/Table.tsx Outdated
Comment thread vuu-ui/packages/vuu-table-types/index.d.ts Outdated

@heswell heswell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should park this. The problem is is intended to solve is better addressed with the solution we discussed offline.

Replace checkboxRowLevelProps with isRowSelectable: (dataRow: DataRow) => boolean on TableProps.

Non-selectable rows are skipped in useSelection (all selection models — checkbox, extended, single, block).
They receive a vuuTableRow-noSelect CSS class which styles the checkbox as visually disabled.
The checkbox checked state is preserved for non-selectable rows even when isSelected is cleared by a subsequent selection.
@anandBalachandar anandBalachandar changed the title feat(table): add checkboxRowLevelProps to Table for per-row checkbox state feat(table): Add prop isRowSelectable Jul 26, 2026
…undo

- tighten componentProps to Record<string, string|number|boolean>
- UndoCellRenderer reads editSession from context; removes functions from componentProps
- deleteSelectedRows guards against double-counting already-deleted keys
- useEditableTable tracks deleteCount via editSession editState event
* When provided on the checkbox column, rows returning false
* are always shown as checked (pending deletion) regardless of isSelected.
*/
isRowSelectable?: (dataRow: DataRow) => boolean;

@heswell heswell Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We have this on tableProps, I don't think we need it on a column as well

});

const isChecked = !!dataRow.isSelected;
const { isRowSelectable } = column;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this is a general rule. It might be the case that a 'soft deleted' row could be assumed to be checked, if selection is being used for delete.
There are definitely other use cases though where we might want to make a row non-selectable based on data, where we would not want the checkbox to appear selected.

we can effect this locally be taking advantage of the noSelect className. eg.if we want the unselectable checkboxes to appear dimmed but checkin in a specific scenarion, we can use local css and the noSelect class to achieve this.

: CheckboxColumnDescriptor(selectionModel, checkboxColumnWidth),
1,
);
if (isRowSelectable) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not needed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is all good but you're dodging the trickiest part which is the block selection logic (see selection-utils)

If user selects a row, then shift selects to select a block, we must not include any rows that are not selectable.
This will mean changing the single block request to one or more block requests that omit the non-selectable rows

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