Skip to content

fix(status-toggle): hide draggable highlight when selected item is disabled#1987

Draft
akashsonune wants to merge 1 commit intomainfrom
fix/status-toggle-hide-draggable-disabled-selection
Draft

fix(status-toggle): hide draggable highlight when selected item is disabled#1987
akashsonune wants to merge 1 commit intomainfrom
fix/status-toggle-hide-draggable-disabled-selection

Conversation

@akashsonune
Copy link
Copy Markdown
Member

@akashsonune akashsonune commented Apr 30, 2026

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:

Code Coverage

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +18 to +20
@if (selectedIndex() !== undefined) {
<div class="visible-toggle-draggable"></div>
}
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.

high

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.

@akashsonune akashsonune changed the title fix(status-toggle): hide draggable highlight when selected item is di… fix(status-toggle): hide draggable highlight when selected item is disabled Apr 30, 2026
@akashsonune
Copy link
Copy Markdown
Member Author

@spike-rabbit what's your take on this?

@dr-itz
Copy link
Copy Markdown
Member

dr-itz commented May 4, 2026

I don't get what this is trying to fix from the description. I cannot see a wrong behaviour in any of our examples

@akashsonune
Copy link
Copy Markdown
Member Author

I don't get what this is trying to fix from the description. I cannot see a wrong behaviour in any of our examples

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)

@dr-itz
Copy link
Copy Markdown
Member

dr-itz commented May 4, 2026

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

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