fix(status-toggle): hide draggable highlight when selected item is disabled#1987
fix(status-toggle): hide draggable highlight when selected item is disabled#1987akashsonune wants to merge 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request modifies the status toggle component template to conditionally render the draggable highlight only when a selection is active. The reviewer identified a UX regression where visual feedback is lost during dragging from an unselected state and noted that the implementation does not handle cases where a selected item becomes disabled. Recommendations include incorporating an isDragging signal into the template condition and updating the selection logic in the TypeScript file to clear the index if a target item is disabled.
| @if (selectedIndex() !== undefined) { | ||
| <div class="visible-toggle-draggable"></div> | ||
| } |
There was a problem hiding this comment.
The current implementation has two issues: 1. UX Regression: The highlight is hidden during dragging if starting from an unselected state, as selectedIndex remains undefined until the drag ends. This removes visual feedback for the user. 2. Incomplete Fix: It does not handle cases where a previously selected item becomes disabled (e.g., via an update to the items input). In selectItem, the logic returns early if the item is disabled, leaving selectedIndex at its previous value. Thus, the highlight remains visible on the now-disabled item. Recommendation: (a) Add an isDragging signal to the component and update the condition to @if (selectedIndex() !== undefined || isDragging()). (b) Update selectItem in the TypeScript file to clear the selection (set selectedIndex to undefined) if the target item is disabled.
|
@spike-rabbit what's your take on this? |
|
I don't get what this is trying to fix from the description. I cannot see a wrong behaviour in any of our examples |
Something like this Where first item is selected one and it is also disabled, the draggable moves to next item (second) |
Ah, now I get it. It's kind of an edge case, but I think when the selected item is disabled, the drag handle should initially still be on the disabled item (it's selected after all), but then the user shouldn't be able to select it again. I.e. grab and move to something else but not back |
When the selected item is disabled one, the highlight moves to next available item. I find it confusing because the next item appears be in inconsistent state, highlighted but not selected, while the original selection was different one.
In general the highlight is applied when the selection happens
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: