Add Breakpoint table column to Spright #2826
Conversation
|
@jattasNI can you give a first pass when you have time? thanks! |
|
|
||
| _Props/Attrs_ | ||
|
|
||
| - `breakpoint-field-name`: string - The field name in the data record that contains the breakpoint state |
There was a problem hiding this comment.
For other table columns, we typically just call the attribute field-name unless we know the column requires data from multiple fields (like the anchor column with href-field-name and label-field-name). Unless you're aware of something coming from the breakpoint column that will require another field, I'd vote to rename this to field-name too.
|
|
||
| _Element Name_ | ||
|
|
||
| - `spright-table-column-breakpoint-cell-view` |
There was a problem hiding this comment.
I forget, did we talk about Spright vs Ok for this component? At the moment I'd lean towards Ok to start with, just to reduce debates about checklist items that you may or may not have bandwidth to do.
|
|
||
| ### Angular Integration | ||
|
|
||
| - Add new table column and events to Angular and Blazor wrappers. |
There was a problem hiding this comment.
nit: feels weird to mention a wrapper for the event but not the rest of the API.
Also, if building in Ok you're welcome to skip the Angular wrapper if you only need Blazor. If you want to go fully complete then you should add a React wrapper too.
| - Add new table column and events to Angular and Blazor wrappers. | |
| - Standard Angular and Blazor wrappers. |
| - **Disabled**: Outlined red circle with slash through it | ||
| - **Hit**: Filled red circle with highlight/border indicating active state within the indicator itself | ||
|
|
||
| Each indicator will have a minimum 24x24 pixel hit target. |
There was a problem hiding this comment.
Do you have an idea how you'll achieve this? What DOM structure you'll use in the template?
I haven't thought about it much but my initial thought would be to use a browser native <button> element that's 24x24 with the icon slotted inside it. A button is preferable to a span/div with a click handler for accessibility. It's preferable to a nimble-button because you can style it more easily.
For the icons you could either add nimble icons for each state you need or you could include inline SVGs directly in the template. I don't have a strong initial preference. But one thing to consider is how the icon should change with theme. Will they have identical color in every theme or will they change with the theme? If they change with the theme do they do so with the same colors as other icons or do they need custom colors?
|
|
||
| #### Group Header View Element | ||
|
|
||
| No new group header view element. |
There was a problem hiding this comment.
If this column type supported grouping we'd need to decide what it would show in the group header row for each state (e.g what text would be displayed, whether a breakpoint icon would appear too).
https://nimble.ni.dev/storybook/?path=/docs/components-table-column-configuration--docs#grouping
But since you say below that the column won't be groupable, that extra detail is unnecssary. Wouldn't hurt to reiterate here or combine this section into the sorting/grouping section.
|
|
||
| The following events from the cell view will be delegated to the column: | ||
|
|
||
| - `click` events from the breakpoint indicator will be delegated as `breakpoint-toggle` events |
There was a problem hiding this comment.
nit: I'd suggest following the API pattern of the menu button column. It doesn't document a click event, just some menu-button-column-toggle events. So I'd suggest calling this breakpoint-column-toggle and removing mention of the click event and its event data.
| - Visual breakpoint indicators with multiple states (Off, Enabled, Disabled, Hit) | ||
| - Click-to-toggle functionality for setting/removing breakpoints | ||
| - Callback mechanisms for client-implemented breakpoint operations (like row highlighting and context menu, as specified in IxD) | ||
| - Keyboard shortcuts for breakpoint management (Ctrl+B, F9) |
There was a problem hiding this comment.
Heads up that keyboard shortcuts might get complicated. For example I'm not sure what would be responsible for delivering the key events to that specific column if the entire row is focused. If this is an ok component we could figure it out in implementation and consider cutting that capability. If this is a spright component then we probably want a little more detail about how this will work in the spec.
2 participants
Pull Request
🤨 Rationale
Clients need a way to provide a breakpoint in the nimble table. This is the implementation of the breakpoint IxD
👩💻 Implementation
N/A
🧪 Testing
N/A
✅ Checklist