feat(ui5-table): add range selection to selection feature#9205
feat(ui5-table): add range selection to selection feature#9205
Conversation
In "Multiple" selection mode, it is now possible to select ranges with a) mouse or b) keyboard. a) Select a row via checkbox and Shift + Click on another checkbox to select the range. b) Select a row and Shift + Up/Down to extend your range selection.
packages/main/src/TableSelection.ts
Outdated
| selectionChanged = true; | ||
| } | ||
|
|
||
| // Workaround required due to bug with mouse, where the checkbox stays unchecked, but the row is selected. |
There was a problem hiding this comment.
this is weird we need to check this
There was a problem hiding this comment.
100% agree. Maybe I'm missing something
There was a problem hiding this comment.
can this be related with the user interaction happens on the checkbox, where user interaction is also using the checked/selected property under the hood.
There was a problem hiding this comment.
Could be the case, yes. Removed the custom handling for click in the Selection feature (and restored it in the TableHeaderRow, etc.) and it seems to work fine now?
packages/main/src/TableSelection.ts
Outdated
| } | ||
|
|
||
| _isSelectionCheckbox(e: Event) { | ||
| return e.composedPath().some((el: EventTarget) => (el as HTMLElement).id === "selection-component"); |
There was a problem hiding this comment.
any element might have an id with selection-component
There was a problem hiding this comment.
Tried to use a unique attribute instead (data-ui5-table-selection-component)
There was a problem hiding this comment.
Feel free to suggest any other ideas
packages/main/src/TableSelection.ts
Outdated
|
|
||
| const focusedElement = getActiveElement(); // Assumption: The focused element is always the "next" row after navigation. | ||
|
|
||
| if (!isInstanceOfTableRow(focusedElement) && !this._rangeSelection?.isMouse) { |
There was a problem hiding this comment.
if I select the last row and shift arrow down and then I am on the growing button should then the range selection stop?
There was a problem hiding this comment.
Tough to say. When you hold down the key, maybe the range selection should continue?
Tried it out in the m.Table and at least there it is not possible to focus the growing button with Arrow keys.
But there is some weird behaviour there.
Maybe range selection should only explicitly stop, when releasing the shift key?
|
|
||
| {{#*inline "growingRow"}} | ||
| <ui5-table-row id="growing-row"> | ||
| <ui5-table-row id="growing-row" ui5-growing-row> |
There was a problem hiding this comment.
what is the reason for that?
we can identify it from its id and it is single for the table.
There was a problem hiding this comment.
Same reasoning, why I've added ui5-table-row-base.
Applications could go ahead an set a random row with id="growing-row" and then, it is not possible to discern our internal growing row from the external one (e.g. in a focusin, keyup event handler)
Very unlikely scenario, but just to be on the safe side and have no ID restrictions. Of course this issue could also occur, if the app is setting the ui5-growing-row attribute, but then I think it is quite malicious from their side.
There was a problem hiding this comment.
for rows I understand but not for the table
what I mean is instead of checking the attribute we can check ` === table._growingRow``
but ok
In "Multiple" selection mode, it is now possible to select ranges with a) mouse or b) keyboard.
a) Select a row via checkbox and Shift + Click on another checkbox to select the range
b) Select a row and Shift + Up/Down to extend your range selection.