Skip to content

table: pass click modifiers through SelectRow event - #2231

Closed
boboshan wants to merge 1 commit into
longbridge:mainfrom
neath-audio:feat/select-row-modifiers
Closed

table: pass click modifiers through SelectRow event#2231
boboshan wants to merge 1 commit into
longbridge:mainfrom
neath-audio:feat/select-row-modifiers

Conversation

@boboshan

@boboshan boboshan commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Fixes #2228

Summary

  • Changes SelectRow(usize) to SelectRow(usize, Modifiers) to carry keyboard modifiers from the click event
  • Adds set_selected_row_with_modifiers() alongside the existing set_selected_row() (which passes default modifiers)
  • Enables consumers to implement Cmd+click / Shift+click multi-row selection without intercepting mouse events on the row div (which breaks on_drag on child elements)

AI Disclosure

This code was generated with AI assistance (Claude). The implementation has been manually tested in a production GPUI application with multi-row selection and native macOS drag.

@huacnlee

huacnlee commented Apr 8, 2026

Copy link
Copy Markdown
Member

Duplicate PR #2232?

@boboshan

boboshan commented Apr 8, 2026

Copy link
Copy Markdown
Contributor Author

Not a duplicate of #2232. These are separate changes:

#2232 depends on this PR because both touch state.rs, but they solve different problems.

SelectRow now carries Modifiers so consumers can implement
Cmd+click / Shift+click multi-select without intercepting
mouse events on the row div (which breaks on_drag on children).

Also adds set_parent_menu() on PopupMenu for external submenu wiring.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@boboshan
boboshan force-pushed the feat/select-row-modifiers branch from 19bb64c to 16646ec Compare April 9, 2026 12:34
@boboshan

Copy link
Copy Markdown
Contributor Author

Closing in favor of using GPUI's existing Window::modifiers() accessor.

Consumers of TableEvent::SelectRow can subscribe with cx.subscribe_in(&table_state, window, ...) to receive &mut Window in the closure, then call window.modifiers() to read the current modifier state at event-handling time. This delivers the same information for Cmd+click / Shift+click multi-selection without changing the event variant or TableDelegate API.

Verified locally — multi-row selection works as expected. Closing #2228 alongside.

@boboshan boboshan closed this Apr 25, 2026
@boboshan
boboshan deleted the feat/select-row-modifiers branch April 25, 2026 17:55
huacnlee added a commit that referenced this pull request Aug 10, 2026
## Summary

Fixes #2229 — three related fixes for submenu dismiss behavior in
`PopupMenu`:

1. **Auto-wire `parent_menu` in `render()`**: Submenus added via
`PopupMenuItem::submenu()` (the data constructor) don't get
`parent_menu` set, unlike `PopupMenu::submenu()` (the builder method).
This breaks the dismiss chain. Now `PopupMenu::render()` auto-wires
`parent_menu` on any submenu child that doesn't have it set. This is
needed because table delegates operate in `Context<TableState>` and
cannot call `PopupMenu::submenu()` which requires `Context<PopupMenu>`.

2. **Fix ESC not closing menus when a submenu is open**: When a submenu
is opened by hover, the parent menu retains focus. Pressing ESC would
hit the parent's `dismiss()`, which bailed out early because
`active_submenu()` was `Some`. Split `dismiss` into `dismiss` (ESC
action handler that clears the active submenu first) and `dismiss_menu`
(internal dismiss that propagates up the chain).

3. **Guard `handle_dismiss` for submenu clicks**: `handle_dismiss`
(click-outside) returns early if a submenu is active. Without this,
clicking a submenu item triggers the parent's `on_mouse_down_out`,
tearing down the submenu before the item's `on_click` fires.

Changes are in `popup_menu.rs` only — no table code changes needed.

No dependency on #2231.

## AI Disclosure

Implementation was developed with AI assistance (Claude). All code has
been reviewed, tested, and follows existing `popup_menu.rs` patterns.

## Test plan

- [x] `cargo run --example table` — right-click context menu with
submenus works
- [x] `cargo run` (story) — Menu story context menu: ESC closes full
chain
- [x] Submenu item click fires handler and dismisses menu
- [x] Click outside closes all menus
- [x] Hover navigation between submenus still works

---------

Co-authored-by: Jason Lee <huacnlee@gmail.com>
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.

table: pass click modifiers through SelectRow event

2 participants