-
Notifications
You must be signed in to change notification settings - Fork 8
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
Editable table cells IxD spec #2477
base: main
Are you sure you want to change the base?
Changes from all commits
22e49f8
33ff2fc
7f04685
9a3ffdf
f3cb122
02a0fd8
c135d02
9d64014
e7118e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "none", | ||
"comment": "IxD spec for editable table", | ||
"packageName": "@ni/nimble-components", | ||
"email": "[email protected]", | ||
"dependentChangeType": "none" | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
# Nimble Editable Table (IxD) | ||
|
||
## Overview | ||
|
||
Nimble table support for interactive cell editing. | ||
|
||
### Background | ||
|
||
- [Figma worksheet](https://www.figma.com/design/r2yGNQNVFdE7cBO9CyHmQx/Nimble---IxD?node-id=1221-36463) | ||
- [Input table columns (select, text field, etc) #1190](https://github.com/ni/nimble/issues/1190) | ||
|
||
## Usage | ||
|
||
**When to use:** | ||
|
||
- When the table is used to display and edit structured engineering data such as measurements, specifications, or configurations. | ||
- When inline editing can improve the user experience by reducing the need for separate forms or dialogs. | ||
- When data accuracy can be ensured through validation and error handling mechanisms. | ||
|
||
**When not to use:** | ||
|
||
- When the table is used for displaying static or read-only data. | ||
- When the user requires guidance entering or modifying data and would be best served by a form or other UX pattern. | ||
- When the application requires spreadsheet features like pasting or filling data into a range of cells | ||
|
||
## Requirements | ||
|
||
- Inline editing support for table cells | ||
- Validation and error handling for input data | ||
- Initiate cell editing via keyboard or mouse | ||
- Support for strings and numbers (1st priority) | ||
- Support for enumerated values (2nd priority) | ||
- All cells within a particular column share the same edit view and configuration | ||
- Compatibility with assistive technologies (e.g., screen readers) | ||
|
||
### Out of scope | ||
|
||
- Support for complex data types (e.g., dates, times, custom objects) | ||
- Advanced spreadsheet functionalities (e.g., formulas, cell references) | ||
- Real-time collaboration or multi-user editing | ||
- Custom cell styling or formatting | ||
|
||
## Anatomy | ||
|
||
![Editable table anatomy](./spec-images/editable-cell-anatomy.png) | ||
|
||
| Element | Description | | ||
| --------------------- | ------------------------------------------------------------------------ | | ||
| Static cell value | Non-editable display of cell value | | ||
| Action button | Button to open cell context menu. Appears on row hover or keyboard focus | | ||
| Editable cell control | Text or numeric control that supports entering or editing cell data | | ||
|
||
## Behavior | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the scope of "editability"?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Specific columns and we'll need to start with text and numeric, and add select and checkbox later. We shouldn't need to support every column type (at least right away.) |
||
|
||
### States | ||
|
||
![Editable cell states for text](./spec-images/text-editable-cell-states.png) | ||
![Editable cell states for numeric](./spec-images/numeric-editable-cell-states.png) | ||
![Editable cell states for select](./spec-images/select-editable-cell-states.png) | ||
![Editable cell states for checkbox](./spec-images/checkbox-editable-cell-states.png) | ||
|
||
#### Error State | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do apps need to be able to do their own validation and explicitly put a cell in an error state? Or is the table completely in control of the error state? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is ultimately a discussion for the HLD for the implementation of this feature. My preference, at the moment, would be for our error APIs to be very general provided via the table, and not by various columns (i.e. the table is not in complete control of the error state, it is managed by the user via APIs provided at the table level). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Requirements-wise: Client apps need to be able to specify their own cell validation rules (e.g., max/min/invalid characters/format) at the column level and they should get that feedback while editing the cell. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a call to essentially surface the appropriate HTML |
||
|
||
![Editable cell error states](./spec-images/editable-cell-error-data.png) | ||
|
||
### ARIA Considerations | ||
|
||
- Maintain compatibility with existing keyboard navigation behavior | ||
- Consider resolving [existing ARIA gaps](https://github.com/ni/nimble/issues/2285) | ||
- Consider adding `aria-readonly` for non-editable cells | ||
|
||
### Mouse Interactions | ||
fredvisser marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we cover mousewheel behavior in this section? Due to the table's virtualization we need to be explicit about the behavior we want. If a cell has Edit focus (potentially with an edited cell value), then the user scrolls the mousewheel, is that equivalent to committing or reverting the edited value? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd strongly prefer that we maintain the edit state even if the cell scrolls out of view, such that it can scroll back into view in the same state. While it's true the menu button dropdown closes on scroll, setting or reverting an edited cell value is a much more aggressive action in response to something as simple as scrolling. Do we have any options for persisting this type of state as the cell is scrolled out of view? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If that proves too expensive, we should prevent the table from scrolling while a cell is in edit focus. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Discussed this a bit with Fred today. Maintaining an edited cell as 'still being edited' after a scroll will probably be difficult due to virtualization - as soon as you scroll more than 1 row's height, even if the row is still visible in the table viewport, the row elements have been re-bound to different data at that point. We can remember which row/cell was being edited (and the current edited cell value), but we'd need to reapply that to a new row visual again (since now a different row visual represents the data row that was being edited, after the scroll), and there might be visual artifacts/ visible flashing if we do that. Assuming we can prevent mousewheel scrolling the table while editing a cell, that's probably the simplest approach (least work to implement). |
||
|
||
![Editable cell mouse navigation](./spec-images/editable-cell-mouse-navigation.png) | ||
|
||
- Editable table cells show the same row hover state as non-editable cells on mouse hover. | ||
- I.e. When hovering over a row, the action menu button appears. Depending on the table's selection mode, the row may be highlighted. | ||
- The cursor will change from an *arrow cursor* when hovering over editable cells | ||
- To a *text cursor* when hovering over cells with editable text or numeric controls | ||
- To a *hand cursor* when hovering over select or boolean controls | ||
- Single clicking editable cell shows the *edit focus* state. | ||
- Clicking away from a focused editable cell sets the value and removes *edit focus*. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be crystal clear, when you click on a non-editable cell while an editable cell has focus, and the row is selectable, do we expect the row to become selected? This is what I expect to happen with no extra work. |
||
|
||
### Keyboard Interactions | ||
|
||
![Editable cell key navigation](./spec-images/editable-cell-key-navigation.png) | ||
|
||
- When a cell has keyboard focus (*cell focus* state), pressing `TAB` or `ENTER` transforms the cell into an input control in the *edit focus* state. | ||
- When a cell has *edit focus*, pressing `TAB` moves the focus to the action button, or to the next available focus target. | ||
|
||
![Editable cell enter key](./spec-images/editable-cell-enter-key.png) | ||
|
||
- When a cell has *edit focus*, pressing `ENTER` sets the value and transforms it into the *cell focus* state. | ||
|
||
![Editable cell escape key](./spec-images/editable-cell-escape-key.png) | ||
|
||
- When a cell has *edit focus*, pressing `ESCAPE` reverts any value change and transforms it into the *cell focus* state. | ||
|
||
![Editable cell data error](./spec-images/editable-cell-error-data.png) | ||
|
||
- User edits are validated on entry. If the entered data is invalid, | ||
- the error is shown in the *edit focus + error* state. | ||
- pressing `TAB`, `ENTER` or `ESC` resets the cell to the most recent valid value. | ||
- Other invalid cell data is shown in the *cell error* state. | ||
|
||
### Out of scope | ||
|
||
The client application is responsible for defining and implementing workflows for adding, deleting, and moving rows, as well as saving data. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably need to think through situations like if editing a value causes its sort order or grouping to change. Do we have a recommended experience? Are we ok with the value you just edited disappearing off screen? Should we scroll to it? Wait to apply changes until they finish a batch of edits? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it would be nice to know what our UX options are here, and some reasoning behind our preferred choice. For instance, here are two different approaches that the Radzen DataGrid provides: What isn't made clear in these examples is how changes made in the table component make their way to the "bound" data (if they do at all). One thing you can see, however, is that changes made to the cells have no affect on their current sort position. This would be a pretty tricky proposition for us to "guarantee" if we liked that behavior. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The least disruptive option is keep the new edited value in the same relative location after completing the edit, even if the column sort would move the row. If that's not possible, and the row needs to immediately sort to the correct position, some affordance to indicate that the row is moving would be helpful. E.g., an animation of the row moving to it's new location. We shouldn't scroll to the new position. |
||
|
||
Example row editing UX: | ||
|
||
![Table row editing](./spec-images/table-row-editing.png) | ||
|
||
Example save workflow: | ||
|
||
![Table save workflow](./spec-images/table-save-workflow.png) | ||
|
||
#### Touch-Screen Devices | ||
|
||
- The edit workflow should support touch screen devices. | ||
|
||
## Open Issues | ||
|
||
See content marked "**QUESTION**" or "**NOTE**". | ||
|
||
## References | ||
|
||
- [AG Grid](https://www.ag-grid.com/example/) | ||
- [Patternfly](https://www.patternfly.org/components/table/react-deprecated/#editable-rows) |
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.
Should we state some non-goals to make it clear we're not trying to replicate a spreadsheet experience? For example:
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.
Added to both "When not to use" and "Out of scope"
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.
After reading the DFF tabular data spec I am more nervous that adding/deleting/reordering rows or columns is out of scope and expected to be implemented by clients. It's clear that they expect those operations to be possible and I think they'll seek our guidance about how to do them if we don't provide them. Re-opening in case we want to revisit this now, but I'm also fine to move it to another venue.