From 79f16faae224a37c02b48306c97cb9bc6f39bab5 Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Thu, 8 May 2025 16:39:01 -0400 Subject: [PATCH 01/15] Maybe dropdown fix? --- primitives/src/dropdown_menu.rs | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index f462343..9fd4199 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -83,15 +83,23 @@ pub fn DropdownMenu(props: DropdownMenuProps) -> Element { }); rsx! { - div { - role: "menu", - "data-state": if open() { "open" } else { "closed" }, - "data-disabled": (props.disabled)(), - - onfocusout: move |_| ctx.set_focus(None), - ..props.attributes, - - {props.children} + div { class: "dropdown-menu-root", + // Backdrop for click-outside-to-close + if open() { + div { + class: "dropdown-menu-backdrop", + onclick: move |_| set_open.call(false), + style: "position: fixed; inset: 0; z-index: 999; background: transparent;", + } + } + div { + role: "menu", + "data-state": if open() { "open" } else { "closed" }, + "data-disabled": (props.disabled)(), + onfocusout: move |_| ctx.set_focus(None), + ..props.attributes, + {props.children} + } } } } From eb50d7a16ef3a469744e1f4fd15f3dcf053483f5 Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Thu, 8 May 2025 16:48:53 -0400 Subject: [PATCH 02/15] on focus out try --- primitives/src/context_menu.rs | 33 ++++++++------------------------- primitives/src/dropdown_menu.rs | 33 +++++++++++++++++---------------- 2 files changed, 25 insertions(+), 41 deletions(-) diff --git a/primitives/src/context_menu.rs b/primitives/src/context_menu.rs index bab8f8b..ce10812 100644 --- a/primitives/src/context_menu.rs +++ b/primitives/src/context_menu.rs @@ -115,28 +115,6 @@ pub fn ContextMenu(props: ContextMenuProps) -> Element { current_focus: Signal::new(None), }); - let handle_click = move |event: Event| { - if open() { - let coords = event.data().client_coordinates(); - let click_x = coords.x as i32; - let click_y = coords.y as i32; - let (menu_x, menu_y) = position(); - - // Simple boundary check - let menu_width = 200; // Approximate menu width - let menu_height = 200; // Approximate menu height - - if click_x < menu_x - || click_x > menu_x + menu_width - || click_y < menu_y - || click_y > menu_y + menu_height - { - set_open.call(false); - ctx.restore_trigger_focus(); - } - } - }; - // Handle escape key to close the menu let handle_keydown = move |event: Event| { if open() && event.key() == Key::Escape { @@ -148,12 +126,17 @@ pub fn ContextMenu(props: ContextMenuProps) -> Element { rsx! { div { - onclick: handle_click, + tabindex: 0, // Make the menu container focusable + onfocusout: move |_| { + if open() { + set_open.call(false); + ctx.restore_trigger_focus(); + } + }, onkeydown: handle_keydown, "data-state": if open() { "open" } else { "closed" }, "data-disabled": (props.disabled)(), ..props.attributes, - {props.children} } } @@ -322,7 +305,7 @@ pub fn ContextMenuItem(props: ContextMenuItemProps) -> Element { div { role: "menuitem", tabindex: tab_index, - onclick: handle_click, + onmousedown: handle_click, onkeydown: handle_keydown, onfocus: move |_| ctx.set_focus(Some((props.index)())), aria_disabled: (ctx.disabled)(), diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index 9fd4199..eb70293 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -83,23 +83,24 @@ pub fn DropdownMenu(props: DropdownMenuProps) -> Element { }); rsx! { - div { class: "dropdown-menu-root", - // Backdrop for click-outside-to-close - if open() { - div { - class: "dropdown-menu-backdrop", - onclick: move |_| set_open.call(false), - style: "position: fixed; inset: 0; z-index: 999; background: transparent;", + div { + tabindex: 0, + onfocusout: move |_| { + if open() { + set_open.call(false); + } + }, + role: "menu", + "data-state": if open() { "open" } else { "closed" }, + "data-disabled": (props.disabled)(), + onkeydown: move |event| { + if event.key() == Key::Escape && open() { + event.prevent_default(); + set_open.call(false); } - } - div { - role: "menu", - "data-state": if open() { "open" } else { "closed" }, - "data-disabled": (props.disabled)(), - onfocusout: move |_| ctx.set_focus(None), - ..props.attributes, - {props.children} - } + }, + ..props.attributes, + {props.children} } } } From d93355b06e0a0f7844fe6afe3921c5e04796a0d5 Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Thu, 8 May 2025 16:52:56 -0400 Subject: [PATCH 03/15] this way might work --- primitives/src/context_menu.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/primitives/src/context_menu.rs b/primitives/src/context_menu.rs index ce10812..a8d9806 100644 --- a/primitives/src/context_menu.rs +++ b/primitives/src/context_menu.rs @@ -125,6 +125,14 @@ pub fn ContextMenu(props: ContextMenuProps) -> Element { }; rsx! { + if open() { + div { + onclick: move |_| { + set_open.call(false); + ctx.restore_trigger_focus(); + }, + } + } div { tabindex: 0, // Make the menu container focusable onfocusout: move |_| { @@ -164,9 +172,18 @@ pub fn ContextMenuTrigger(props: ContextMenuTriggerProps) -> Element { } }; + // NEW: Handle left-click to close if already open + let handle_click = move |_| { + if (ctx.open)() { + ctx.set_open.call(false); + ctx.restore_trigger_focus(); + } + }; + rsx! { div { oncontextmenu: handle_context_menu, + onclick: handle_click, aria_haspopup: "menu", aria_expanded: (ctx.open)(), ..props.attributes, From 9d6065f7bdb1b05e45d2783db8db538550bec643 Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Thu, 15 May 2025 11:57:23 -0400 Subject: [PATCH 04/15] item selection should work now --- preview/src/main.rs | 15 ++++++-- primitives/src/dropdown_menu.rs | 63 ++++++++++++++++++++++++++++----- 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/preview/src/main.rs b/preview/src/main.rs index fd03357..892da2a 100644 --- a/preview/src/main.rs +++ b/preview/src/main.rs @@ -756,6 +756,7 @@ fn TabsExample() -> Element { #[component] fn DropdownMenuExample() -> Element { + let mut selected_value = use_signal(String::new); rsx! { document::Link { rel: "stylesheet", href: asset!("./assets/dropdown-menu.css") } DropdownMenu { class: "dropdown-menu", default_open: false, @@ -769,7 +770,7 @@ fn DropdownMenuExample() -> Element { value: ReadOnlySignal::new(Signal::new("item1".to_string())), index: ReadOnlySignal::new(Signal::new(0)), on_select: move |value| { - eval(&format!("console.log('Selected: {}')", value)); + selected_value.set(value); }, "Item 1" } @@ -779,7 +780,7 @@ fn DropdownMenuExample() -> Element { value: ReadOnlySignal::new(Signal::new("item2".to_string())), index: ReadOnlySignal::new(Signal::new(1)), on_select: move |value| { - eval(&format!("console.log('Selected: {}')", value)); + selected_value.set(value); }, "Item 2" } @@ -789,12 +790,20 @@ fn DropdownMenuExample() -> Element { value: ReadOnlySignal::new(Signal::new("item3".to_string())), index: ReadOnlySignal::new(Signal::new(2)), on_select: move |value| { - eval(&format!("console.log('Selected: {}')", value)); + selected_value.set(value); }, "Item 3" } } } + + div { class: "selected-value", + if selected_value().is_empty() { + "No selection" + } else { + "Selected: {selected_value()}" + } + } } } diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index eb70293..ebcf738 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -82,23 +82,39 @@ pub fn DropdownMenu(props: DropdownMenuProps) -> Element { current_focus: Signal::new(None), }); + // Use a signal to track if we're in a click operation + let mut is_selecting = use_signal(|| false); + + // Handle escape key to close the menu + let handle_keydown = move |event: Event| { + if open() && event.key() == Key::Escape { + event.prevent_default(); + set_open.call(false); + ctx.set_focus(None); + } + }; + rsx! { div { tabindex: 0, + // We only close on focusout if we're not in a selection action onfocusout: move |_| { - if open() { + if open() && !is_selecting() && !is_selecting() { set_open.call(false); } }, role: "menu", "data-state": if open() { "open" } else { "closed" }, "data-disabled": (props.disabled)(), - onkeydown: move |event| { - if event.key() == Key::Escape && open() { - event.prevent_default(); - set_open.call(false); - } + // Track mousedown events for selection + onmousedown: move |_| { + is_selecting.set(true); }, + // Reset selection flag on mouseup + onmouseup: move |_| { + is_selecting.set(false); + }, + onkeydown: handle_keydown, ..props.attributes, {props.children} } @@ -145,15 +161,43 @@ pub struct DropdownMenuContentProps { #[component] pub fn DropdownMenuContent(props: DropdownMenuContentProps) -> Element { - let ctx: DropdownMenuContext = use_context(); + let mut ctx: DropdownMenuContext = use_context(); let open = ctx.open; + // When menu opens, focus the first item + let is_open = open(); + use_effect(move || { + if is_open { + ctx.focus_first(); + } + }); + rsx! { div { role: "menu", "data-state": if open() { "open" } else { "closed" }, hidden: !open(), - + // Stop propagation to prevent unwanted interactions + onclick: move |e| { + e.stop_propagation(); + }, + onkeydown: move |event: Event| { + let mut prevent_default = true; + match event.key() { + Key::ArrowDown => ctx.focus_next(), + Key::ArrowUp => ctx.focus_prev(), + Key::Home => ctx.focus_first(), + Key::End => ctx.focus_last(), + Key::Escape => { + ctx.set_open.call(false); + ctx.set_focus(None); + } + _ => prevent_default = false, + } + if prevent_default { + event.prevent_default(); + } + }, ..props.attributes, {props.children} } @@ -208,7 +252,8 @@ pub fn DropdownMenuItem(props: DropdownMenuItemProps) -> Element { onclick: { let value = (props.value)().clone(); - move |_| { + move |e| { + e.stop_propagation(); if !(ctx.disabled)() && !(props.disabled)() { props.on_select.call(value.clone()); ctx.set_open.call(false); From 78f2d75fabca3670108b70541450796566830520 Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Sat, 17 May 2025 10:17:52 -0400 Subject: [PATCH 05/15] adding type --- primitives/src/dropdown_menu.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index ebcf738..22c83d8 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -252,7 +252,7 @@ pub fn DropdownMenuItem(props: DropdownMenuItemProps) -> Element { onclick: { let value = (props.value)().clone(); - move |e| { + move |e: Event| { e.stop_propagation(); if !(ctx.disabled)() && !(props.disabled)() { props.on_select.call(value.clone()); From afebe2bd55530a734b97565b557689cd36af8888 Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Wed, 28 May 2025 17:27:22 -0400 Subject: [PATCH 06/15] Focus out try for menubar --- primitives/src/menubar.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/primitives/src/menubar.rs b/primitives/src/menubar.rs index 8536b6f..960e198 100644 --- a/primitives/src/menubar.rs +++ b/primitives/src/menubar.rs @@ -69,12 +69,25 @@ pub fn Menubar(props: MenubarProps) -> Element { current_focus: Signal::new(None), }); + // Add is_selecting signal to track if a click is happening inside + let mut is_selecting = use_signal(|| false); + rsx! { div { role: "menubar", "data-disabled": (props.disabled)(), - onfocusout: move |_| ctx.set_focus(None), + // Set is_selecting on mousedown/mouseup + onmousedown: move |_| is_selecting.set(true), + onmouseup: move |_| is_selecting.set(false), + + // Close menu on focusout if not selecting + onfocusout: move |_| { + ctx.set_focus(None); + if open_menu().is_some() && !is_selecting() { + set_open_menu.call(None); + } + }, ..props.attributes, {props.children} @@ -118,6 +131,13 @@ pub fn MenubarMenu(props: MenubarMenuProps) -> Element { } }); + // Focus the first item when the menu is opened + use_effect(move || { + if is_open() { + ctx.focus_first(); + } + }); + rsx! { div { role: "menu", From f27c85d0ec9351d49e0fbebf1d2561b5d8054acc Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Sat, 31 May 2025 14:00:39 -0400 Subject: [PATCH 07/15] Using data state instead of hidden and basic animations --- preview/src/components/context_menu/style.css | 30 +++++++++-------- .../src/components/dropdown_menu/style.css | 20 +++++++++-- preview/src/components/menubar/style.css | 25 +++++++++++--- primitives/src/context_menu.rs | 1 - primitives/src/dropdown_menu.rs | 1 - primitives/src/menubar.rs | 33 ++++++++++++++++--- 6 files changed, 82 insertions(+), 28 deletions(-) diff --git a/preview/src/components/context_menu/style.css b/preview/src/components/context_menu/style.css index db57e10..392750b 100644 --- a/preview/src/components/context_menu/style.css +++ b/preview/src/components/context_menu/style.css @@ -18,11 +18,22 @@ padding: 5px; box-shadow: 0 10px 20px rgba(0, 0, 0, 0.1); border: 1px solid var(--dim-border-color); - animation: slideIn 0.1s ease-out; + z-index: 1000; + transform-origin: var(--radix-context-menu-content-transform-origin); + will-change: transform, opacity; } -.context-menu-content[hidden] { - display: none; +.context-menu-content[data-state="closed"] { + pointer-events: none; + opacity: 0; + transform: scale(0.95) translateY(-2px); + transition: opacity 150ms ease-in, transform 150ms ease-in; +} + +.context-menu-content[data-state="open"] { + opacity: 1; + transform: scale(1) translateY(0); + transition: opacity 200ms ease-out, transform 200ms cubic-bezier(0.16, 1, 0.3, 1); } .context-menu-item { @@ -35,6 +46,7 @@ color: var(--text-color); display: flex; align-items: center; + transition: background-color 100ms ease-out; } .context-menu-item:hover { @@ -45,14 +57,4 @@ background: var(--focused-background-color); } -@keyframes slideIn { - from { - opacity: 0; - transform: scale(0.95); - } - - to { - opacity: 1; - transform: scale(1); - } -} \ No newline at end of file +/* Remove old keyframes - now using CSS transitions for smoother animations */ \ No newline at end of file diff --git a/preview/src/components/dropdown_menu/style.css b/preview/src/components/dropdown_menu/style.css index dd102d2..e62e478 100644 --- a/preview/src/components/dropdown_menu/style.css +++ b/preview/src/components/dropdown_menu/style.css @@ -36,10 +36,26 @@ box-shadow: 0 2px 8px rgba(0, 0, 0, 0.15); z-index: 1000; padding: 4px 0; + + /* Animation properties */ + opacity: 0; + transform: translateY(-8px) scale(0.95); + transition: all 0.2s ease; + pointer-events: none; +} + +/* Open state animations */ +.dropdown-menu-content[data-state="open"] { + opacity: 1; + transform: translateY(0) scale(1); + pointer-events: auto; } -.dropdown-menu-content[hidden] { - display: none; +/* Closed state animations */ +.dropdown-menu-content[data-state="closed"] { + opacity: 0; + transform: translateY(-8px) scale(0.95); + pointer-events: none; } .dropdown-menu-item { diff --git a/preview/src/components/menubar/style.css b/preview/src/components/menubar/style.css index e45ee7c..bf04d11 100644 --- a/preview/src/components/menubar/style.css +++ b/preview/src/components/menubar/style.css @@ -17,15 +17,15 @@ background: none; cursor: pointer; color: var(--text-color); + border-radius: 4px; + transition: background-color 100ms ease-out; } .menubar-trigger:hover { background: var(--hover-background-color); - border-radius: 4px; } .menubar-content { - display: none; position: absolute; top: 100%; left: 0; @@ -35,10 +35,22 @@ border-radius: 4px; padding: 4px; box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1); + z-index: 1000; + transform-origin: top; + will-change: transform, opacity; } -.menubar-menu[data-state="open"] .menubar-content { - display: block; +.menubar-content[data-state="closed"] { + pointer-events: none; + opacity: 0; + transform: translateY(-4px) scale(0.98); + transition: opacity 150ms ease-in, transform 150ms ease-in; +} + +.menubar-content[data-state="open"] { + opacity: 1; + transform: translateY(0) scale(1); + transition: opacity 200ms ease-out, transform 200ms cubic-bezier(0.16, 1, 0.3, 1); } .menubar-item { @@ -46,6 +58,7 @@ padding: 8px 12px; cursor: pointer; border-radius: 4px; + transition: background-color 100ms ease-out; } .menubar-item:hover { @@ -55,4 +68,6 @@ [data-disabled="true"] { opacity: 0.5; cursor: not-allowed; -} \ No newline at end of file +} + +/* Remove old keyframes - now using CSS transitions for smoother animations */ \ No newline at end of file diff --git a/primitives/src/context_menu.rs b/primitives/src/context_menu.rs index ce8c35c..277953b 100644 --- a/primitives/src/context_menu.rs +++ b/primitives/src/context_menu.rs @@ -223,7 +223,6 @@ pub fn ContextMenuContent(props: ContextMenuContentProps) -> Element { aria_orientation: "vertical", style: "{style}", "data-state": if (ctx.open)() { "open" } else { "closed" }, - hidden: !(ctx.open)(), onclick: move |e| e.stop_propagation(), onkeydown: move |event: Event| { let mut prevent_default = true; diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index 8c93156..407e48d 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -176,7 +176,6 @@ pub fn DropdownMenuContent(props: DropdownMenuContentProps) -> Element { div { role: "menu", "data-state": if open() { "open" } else { "closed" }, - hidden: !open(), // Stop propagation to prevent unwanted interactions onclick: move |e| { e.stop_propagation(); diff --git a/primitives/src/menubar.rs b/primitives/src/menubar.rs index 052242e..c6ead63 100644 --- a/primitives/src/menubar.rs +++ b/primitives/src/menubar.rs @@ -15,6 +15,14 @@ struct MenubarContext { current_focus: Signal>, } +#[derive(Clone, Copy)] +struct MenubarMenuContext { + // The index of this specific menu + menu_index: usize, + // Reference to the global menubar context + global_ctx: MenubarContext, +} + impl MenubarContext { fn set_focus(&mut self, index: Option) { if let Some(idx) = index { @@ -140,6 +148,12 @@ pub fn MenubarMenu(props: MenubarMenuProps) -> Element { } }); + // Provide the menu-specific context + let _menu_ctx = use_context_provider(|| MenubarMenuContext { + menu_index: props.index, + global_ctx: ctx, + }); + rsx! { div { role: "menu", @@ -206,8 +220,17 @@ pub struct MenubarContentProps { #[component] pub fn MenubarContent(props: MenubarContentProps) -> Element { + let menu_ctx: MenubarMenuContext = use_context(); + let is_open = use_memo(move || (menu_ctx.global_ctx.open_menu)() == Some(menu_ctx.menu_index)); + rsx! { - div { role: "menu", ..props.attributes, {props.children} } + div { + role: "menu", + "data-state": if is_open() { "open" } else { "closed" }, + onclick: move |e| e.stop_propagation(), + ..props.attributes, + {props.children} + } } } @@ -228,19 +251,19 @@ pub struct MenubarItemProps { #[component] pub fn MenubarItem(props: MenubarItemProps) -> Element { - let ctx: MenubarContext = use_context(); + let menu_ctx: MenubarMenuContext = use_context(); rsx! { div { role: "menuitem", - "data-disabled": (ctx.disabled)() || (props.disabled)(), + "data-disabled": (menu_ctx.global_ctx.disabled)() || (props.disabled)(), onclick: { let value = props.value.clone(); move |_| { - if !(ctx.disabled)() && !(props.disabled)() { + if !(menu_ctx.global_ctx.disabled)() && !(props.disabled)() { props.on_select.call(value.clone()); - ctx.set_open_menu.call(None); + menu_ctx.global_ctx.set_open_menu.call(None); } } }, From a8e41a2c346882dc071292cd51c034dfb0c29279 Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Mon, 2 Jun 2025 09:51:10 -0400 Subject: [PATCH 08/15] Adding inert to context_menu, dropdown menu and menubar --- primitives/src/context_menu.rs | 1 + primitives/src/dropdown_menu.rs | 1 + primitives/src/menubar.rs | 7 ++++--- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/primitives/src/context_menu.rs b/primitives/src/context_menu.rs index 277953b..b573722 100644 --- a/primitives/src/context_menu.rs +++ b/primitives/src/context_menu.rs @@ -223,6 +223,7 @@ pub fn ContextMenuContent(props: ContextMenuContentProps) -> Element { aria_orientation: "vertical", style: "{style}", "data-state": if (ctx.open)() { "open" } else { "closed" }, + "inert": if !(ctx.open)() { "true" } else { "" }, onclick: move |e| e.stop_propagation(), onkeydown: move |event: Event| { let mut prevent_default = true; diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index 407e48d..cf14ba4 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -176,6 +176,7 @@ pub fn DropdownMenuContent(props: DropdownMenuContentProps) -> Element { div { role: "menu", "data-state": if open() { "open" } else { "closed" }, + "inert": if !open() { "true" } else { "" }, // Stop propagation to prevent unwanted interactions onclick: move |e| { e.stop_propagation(); diff --git a/primitives/src/menubar.rs b/primitives/src/menubar.rs index c6ead63..ae6b165 100644 --- a/primitives/src/menubar.rs +++ b/primitives/src/menubar.rs @@ -224,12 +224,13 @@ pub fn MenubarContent(props: MenubarContentProps) -> Element { let is_open = use_memo(move || (menu_ctx.global_ctx.open_menu)() == Some(menu_ctx.menu_index)); rsx! { - div { + div { role: "menu", "data-state": if is_open() { "open" } else { "closed" }, + "inert": if !is_open() { "true" } else { "" }, onclick: move |e| e.stop_propagation(), - ..props.attributes, - {props.children} + ..props.attributes, + {props.children} } } } From 6d3ae0eebb64d380969944dd997153e5027604e8 Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Mon, 2 Jun 2025 19:25:27 -0400 Subject: [PATCH 09/15] Revert "Adding inert to context_menu, dropdown menu and menubar" This reverts commit a8e41a2c346882dc071292cd51c034dfb0c29279. --- primitives/src/context_menu.rs | 1 - primitives/src/dropdown_menu.rs | 1 - primitives/src/menubar.rs | 7 +++---- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/primitives/src/context_menu.rs b/primitives/src/context_menu.rs index b573722..277953b 100644 --- a/primitives/src/context_menu.rs +++ b/primitives/src/context_menu.rs @@ -223,7 +223,6 @@ pub fn ContextMenuContent(props: ContextMenuContentProps) -> Element { aria_orientation: "vertical", style: "{style}", "data-state": if (ctx.open)() { "open" } else { "closed" }, - "inert": if !(ctx.open)() { "true" } else { "" }, onclick: move |e| e.stop_propagation(), onkeydown: move |event: Event| { let mut prevent_default = true; diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index cf14ba4..407e48d 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -176,7 +176,6 @@ pub fn DropdownMenuContent(props: DropdownMenuContentProps) -> Element { div { role: "menu", "data-state": if open() { "open" } else { "closed" }, - "inert": if !open() { "true" } else { "" }, // Stop propagation to prevent unwanted interactions onclick: move |e| { e.stop_propagation(); diff --git a/primitives/src/menubar.rs b/primitives/src/menubar.rs index ae6b165..c6ead63 100644 --- a/primitives/src/menubar.rs +++ b/primitives/src/menubar.rs @@ -224,13 +224,12 @@ pub fn MenubarContent(props: MenubarContentProps) -> Element { let is_open = use_memo(move || (menu_ctx.global_ctx.open_menu)() == Some(menu_ctx.menu_index)); rsx! { - div { + div { role: "menu", "data-state": if is_open() { "open" } else { "closed" }, - "inert": if !is_open() { "true" } else { "" }, onclick: move |e| e.stop_propagation(), - ..props.attributes, - {props.children} + ..props.attributes, + {props.children} } } } From a12dacb248734f4f0acd51690c0e066c9ef3332f Mon Sep 17 00:00:00 2001 From: Sabin Regmi Date: Mon, 2 Jun 2025 19:41:44 -0400 Subject: [PATCH 10/15] Just comment inert for now --- primitives/src/context_menu.rs | 1 + primitives/src/dropdown_menu.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/primitives/src/context_menu.rs b/primitives/src/context_menu.rs index 277953b..ae3cc59 100644 --- a/primitives/src/context_menu.rs +++ b/primitives/src/context_menu.rs @@ -223,6 +223,7 @@ pub fn ContextMenuContent(props: ContextMenuContentProps) -> Element { aria_orientation: "vertical", style: "{style}", "data-state": if (ctx.open)() { "open" } else { "closed" }, + // "inert": !(ctx.open)(), onclick: move |e| e.stop_propagation(), onkeydown: move |event: Event| { let mut prevent_default = true; diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index 407e48d..30479f0 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -176,6 +176,7 @@ pub fn DropdownMenuContent(props: DropdownMenuContentProps) -> Element { div { role: "menu", "data-state": if open() { "open" } else { "closed" }, + // "inert": !open(), // Stop propagation to prevent unwanted interactions onclick: move |e| { e.stop_propagation(); From cfbaaf08a54ed35d21d45f2e8e34715bd487fd9c Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 11 Jun 2025 13:24:51 -0500 Subject: [PATCH 11/15] handle menubar navigation --- preview/src/components/menubar/mod.rs | 6 + preview/src/components/menubar/style.css | 2 + primitives/src/menubar.rs | 166 +++++++++++++++++++---- 3 files changed, 147 insertions(+), 27 deletions(-) diff --git a/preview/src/components/menubar/mod.rs b/preview/src/components/menubar/mod.rs index ccf86e1..d6394b5 100644 --- a/preview/src/components/menubar/mod.rs +++ b/preview/src/components/menubar/mod.rs @@ -15,6 +15,7 @@ pub(super) fn Demo() -> Element { MenubarTrigger { class: "menubar-trigger", "File" } MenubarContent { class: "menubar-content", MenubarItem { + index: 0usize, class: "menubar-item", value: "new".to_string(), on_select: move |value| { @@ -23,6 +24,7 @@ pub(super) fn Demo() -> Element { "New" } MenubarItem { + index: 1usize, class: "menubar-item", value: "open".to_string(), on_select: move |value| { @@ -31,6 +33,7 @@ pub(super) fn Demo() -> Element { "Open" } MenubarItem { + index: 2usize, class: "menubar-item", value: "save".to_string(), on_select: move |value| { @@ -44,6 +47,7 @@ pub(super) fn Demo() -> Element { MenubarTrigger { class: "menubar-trigger", "Edit" } MenubarContent { class: "menubar-content", MenubarItem { + index: 0usize, class: "menubar-item", value: "cut".to_string(), on_select: move |value| { @@ -52,6 +56,7 @@ pub(super) fn Demo() -> Element { "Cut" } MenubarItem { + index: 1usize, class: "menubar-item", value: "copy".to_string(), on_select: move |value| { @@ -60,6 +65,7 @@ pub(super) fn Demo() -> Element { "Copy" } MenubarItem { + index: 2usize, class: "menubar-item", value: "paste".to_string(), on_select: move |value| { diff --git a/preview/src/components/menubar/style.css b/preview/src/components/menubar/style.css index d0204ff..6dcfccb 100644 --- a/preview/src/components/menubar/style.css +++ b/preview/src/components/menubar/style.css @@ -41,6 +41,7 @@ .menubar-trigger:focus-visible { color: var(--bright-text-color); background: var(--hover-background-color); + outline: none; } @media (prefers-color-scheme: dark) { .menubar-trigger:hover:not([data-disabled="true"]), @@ -92,6 +93,7 @@ .menubar-item:focus-visible { color: var(--bright-text-color); background: var(--hover-background-color); + outline: none; } @media (prefers-color-scheme: dark) { .menubar-item:hover:not([data-disabled="true"]), diff --git a/primitives/src/menubar.rs b/primitives/src/menubar.rs index da52e60..2a271ec 100644 --- a/primitives/src/menubar.rs +++ b/primitives/src/menubar.rs @@ -21,6 +21,9 @@ impl MenubarContext { self.recent_focus.set(idx); } self.current_focus.set(index); + if self.open_menu.read().is_some() { + self.set_open_menu.call(index); + } } fn focus_next(&mut self) { @@ -62,7 +65,7 @@ pub fn Menubar(props: MenubarProps) -> Element { let mut open_menu = use_signal(|| None); let set_open_menu = use_callback(move |idx| open_menu.set(idx)); - let mut ctx = use_context_provider(|| MenubarContext { + use_context_provider(|| MenubarContext { open_menu, set_open_menu, disabled: props.disabled, @@ -76,7 +79,6 @@ pub fn Menubar(props: MenubarProps) -> Element { role: "menubar", "data-disabled": (props.disabled)(), - onfocusout: move |_| ctx.set_focus(None), ..props.attributes, {props.children} @@ -84,6 +86,34 @@ pub fn Menubar(props: MenubarProps) -> Element { } } +#[derive(Clone, Copy)] +struct MenubarMenuContext { + index: usize, + current_focus: Signal>, + item_count: Signal, + is_open: Memo, +} + +impl MenubarMenuContext { + fn focus_next(&mut self) { + let mut current_focus = self.current_focus.write(); + match &mut *current_focus { + Some(current_focus) => *current_focus = (*current_focus + 1) % self.item_count.cloned(), + None => *current_focus = Some(0), + } + } + + fn focus_prev(&mut self) { + let mut current_focus = self.current_focus.write(); + let item_count = self.item_count.cloned(); + match &mut *current_focus { + Some(current_focus) if *current_focus > 0 => *current_focus -= 1, + Some(current_focus) => *current_focus = item_count - 1, + None => *current_focus = Some(item_count - 1), + } + } +} + #[derive(Props, Clone, PartialEq)] pub struct MenubarMenuProps { index: usize, @@ -99,6 +129,19 @@ pub struct MenubarMenuProps { #[component] pub fn MenubarMenu(props: MenubarMenuProps) -> Element { let mut ctx: MenubarContext = use_context(); + let is_open = use_memo(move || (ctx.open_menu)() == Some(props.index)); + let mut menu_ctx = use_context_provider(|| MenubarMenuContext { + index: props.index, + current_focus: Signal::new(None), + item_count: Signal::new(0), + is_open, + }); + + use_effect(move || { + if !is_open() { + menu_ctx.current_focus.set(None); + } + }); use_effect(move || { ctx.menu_count += 1; @@ -111,50 +154,52 @@ pub fn MenubarMenu(props: MenubarMenuProps) -> Element { } }); - let is_open = use_memo(move || (ctx.open_menu)() == Some(props.index)); - let tab_index = use_memo(move || { - if (ctx.current_focus)() == Some(props.index) { - "0" - } else { - "-1" - } - }); + let disabled = move || (ctx.disabled)() || (props.disabled)(); rsx! { div { role: "menu", - tabindex: tab_index, "data-state": if is_open() { "open" } else { "closed" }, "data-disabled": (ctx.disabled)() || (props.disabled)(), onclick: move |_| { - if !(ctx.disabled)() && !(props.disabled)() { + if !disabled() { let new_open = if is_open() { None } else { Some(props.index) }; ctx.set_open_menu.call(new_open); } }, - onfocus: move |_| ctx.set_focus(Some(props.index)), + onpointerenter: move |_| { + if !disabled() && (ctx.open_menu)().is_some() { + ctx.set_focus(Some(props.index)); + } + }, onkeydown: move |event: Event| { - let mut prevent_default = true; match event.key() { - Key::Enter => { - if !(ctx.disabled)() && !(props.disabled)() { - let new_open = if is_open() { None } else { Some(props.index) }; - ctx.set_open_menu.call(new_open); - } + Key::Enter if !disabled() => { + ctx.set_open_menu.call((!is_open()).then_some(props.index)); } Key::Escape => ctx.set_open_menu.call(None), Key::ArrowLeft => ctx.focus_prev(), Key::ArrowRight => ctx.focus_next(), + Key::ArrowDown if !disabled() => { + if is_open() { + menu_ctx.focus_next(); + } else { + ctx.set_open_menu.call(Some(props.index)); + } + }, + Key::ArrowUp if !disabled() => { + if is_open() { + menu_ctx.focus_prev(); + } + }, Key::Home => ctx.focus_first(), Key::End => ctx.focus_last(), - _ => prevent_default = false, - } - if prevent_default { - event.prevent_default(); + _ => return, } + event.prevent_default(); }, ..props.attributes, @@ -172,8 +217,37 @@ pub struct MenubarTriggerProps { #[component] pub fn MenubarTrigger(props: MenubarTriggerProps) -> Element { + let mut ctx: MenubarContext = use_context(); + let menu_ctx: MenubarMenuContext = use_context(); + let mut button_ref: Signal>> = use_signal(|| None); + use_effect(move || { + let Some(button) = button_ref() else { + return; + }; + if (ctx.current_focus)() == Some(menu_ctx.index) { + spawn(async move { + _ = button.set_focus(true).await; + }); + } + }); + rsx! { - button { ..props.attributes,{props.children} } + button { + onmounted: move |node| { + button_ref.set(Some(node.data())); + }, + onfocus: move |_| ctx.set_focus(Some(menu_ctx.index)), + onblur: move |_| { + if (ctx.current_focus)() == Some(menu_ctx.index) && menu_ctx.current_focus.read().is_none() { + tracing::info!("Blur on menu {}", menu_ctx.index); + ctx.set_focus(None); + } + }, + role: "menuitem", + tabindex: if (ctx.recent_focus)() == menu_ctx.index { "0" } else { "-1" }, + ..props.attributes, + {props.children} + } } } @@ -193,6 +267,8 @@ pub fn MenubarContent(props: MenubarContentProps) -> Element { #[derive(Props, Clone, PartialEq)] pub struct MenubarItemProps { + index: usize, + value: String, #[props(default)] @@ -208,22 +284,58 @@ pub struct MenubarItemProps { #[component] pub fn MenubarItem(props: MenubarItemProps) -> Element { - let ctx: MenubarContext = use_context(); + let mut ctx: MenubarContext = use_context(); + let mut menu_ctx: MenubarMenuContext = use_context(); + + use_effect(move || { + menu_ctx.item_count += 1; + }); + use_effect_cleanup(move || { + menu_ctx.item_count -= 1; + if (menu_ctx.current_focus)() == Some(props.index) { + menu_ctx.current_focus.set(None); + } + }); + + let disabled = move || (ctx.disabled)() || (props.disabled)(); + let focused = move || (menu_ctx.current_focus)() == Some(props.index) && (menu_ctx.is_open)(); + + let mut item_ref: Signal>> = use_signal(|| None); + use_effect(move || { + let Some(item) = item_ref() else { + return; + }; + if focused() { + spawn(async move { + _ = item.set_focus(true).await; + }); + } + }); rsx! { div { role: "menuitem", - "data-disabled": (ctx.disabled)() || (props.disabled)(), + "data-disabled": disabled(), + tabindex: if focused() { "0" } else { "-1" }, onclick: { let value = props.value.clone(); move |_| { - if !(ctx.disabled)() && !(props.disabled)() { + if !disabled() { props.on_select.call(value.clone()); ctx.set_open_menu.call(None); } } }, + onmounted: move |node| { + item_ref.set(Some(node.data())); + }, + onblur: move |_| { + if focused() { + menu_ctx.current_focus.set(None); + ctx.set_focus(None); + } + }, ..props.attributes, {props.children} From 342d973df50ee80f0b57e6660ac6373598eb124a Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 11 Jun 2025 13:48:46 -0500 Subject: [PATCH 12/15] merge styles --- preview/src/components/context_menu/style.css | 31 ++++++++++--------- .../src/components/dropdown_menu/style.css | 31 ++++++++++--------- preview/src/components/menubar/style.css | 17 +++------- 3 files changed, 37 insertions(+), 42 deletions(-) diff --git a/preview/src/components/context_menu/style.css b/preview/src/components/context_menu/style.css index add57a5..c1f376f 100644 --- a/preview/src/components/context_menu/style.css +++ b/preview/src/components/context_menu/style.css @@ -4,7 +4,9 @@ border-radius: 0.5rem; padding: 0.25rem; box-shadow: 0 0 0 1px var(--dim-border-color); - animation: slideIn 0.1s ease-out; + z-index: 1000; + transform-origin: var(--radix-context-menu-content-transform-origin); + will-change: transform, opacity; } @media (prefers-color-scheme: dark) { @@ -14,8 +16,18 @@ } } -.context-menu-content[hidden] { - display: none; +.context-menu-content[data-state="closed"] { + pointer-events: none; + opacity: 0; + transform: scale(0.95) translateY(-2px); + transition: opacity 150ms ease-in, transform 150ms ease-in; +} + +.context-menu-content[data-state="open"] { + opacity: 1; + transform: scale(1) translateY(0); + transition: opacity 200ms ease-out, + transform 200ms cubic-bezier(0.16, 1, 0.3, 1); } .context-menu-item { @@ -28,6 +40,7 @@ color: var(--text-color); display: flex; align-items: center; + transition: background-color 100ms ease-out; } .context-menu-item[data-disabled="true"] { @@ -47,15 +60,3 @@ background: var(--hover-border-color); } } - -@keyframes slideIn { - from { - opacity: 0; - transform: scale(0.95); - } - - to { - opacity: 1; - transform: scale(1); - } -} diff --git a/preview/src/components/dropdown_menu/style.css b/preview/src/components/dropdown_menu/style.css index 3fa8a24..da55a05 100644 --- a/preview/src/components/dropdown_menu/style.css +++ b/preview/src/components/dropdown_menu/style.css @@ -42,6 +42,11 @@ padding: 0.25rem; box-shadow: inset 0 0 0 1px var(--dim-border-color); animation: slideIn 0.1s ease-out; + /* Animation properties */ + opacity: 0; + transform: translateY(-8px) scale(0.95); + transition: all 0.2s ease; + pointer-events: none; } @media (prefers-color-scheme: dark) { @@ -51,8 +56,18 @@ } } -.dropdown-menu-content[hidden] { - display: none; +/* Open state animations */ +.dropdown-menu-content[data-state="open"] { + opacity: 1; + transform: translateY(0) scale(1); + pointer-events: auto; +} + +/* Closed state animations */ +.dropdown-menu-content[data-state="closed"] { + opacity: 0; + transform: translateY(-8px) scale(0.95); + pointer-events: none; } .dropdown-menu-item { @@ -84,15 +99,3 @@ background: var(--hover-border-color); } } - -@keyframes slideIn { - from { - opacity: 0; - transform: scale(0.95); - } - - to { - opacity: 1; - transform: scale(1); - } -} diff --git a/preview/src/components/menubar/style.css b/preview/src/components/menubar/style.css index 5591ff3..8c92455 100644 --- a/preview/src/components/menubar/style.css +++ b/preview/src/components/menubar/style.css @@ -20,6 +20,7 @@ cursor: pointer; color: var(--text-color); border-radius: calc(.5rem - .25rem); + transition: background-color 100ms ease-out; } .menubar-menu[data-state="open"] .menubar-trigger { @@ -60,7 +61,9 @@ border-radius: .5rem; padding: .25rem; box-shadow: inset 0 0 0 1px var(--dim-border-color); - animation: menubarSlideIn 0.1s ease-out; + z-index: 1000; + transform-origin: top; + will-change: transform, opacity; } @media (prefers-color-scheme: dark) { @@ -112,15 +115,3 @@ opacity: 0.5; cursor: not-allowed; } - -@keyframes menubarSlideIn { - from { - opacity: 0; - transform: scale(0.95); - } - - to { - opacity: 1; - transform: scale(1); - } -} From ce00340cea20153de97a4a781d2f5ea014c4e56e Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 11 Jun 2025 14:04:23 -0500 Subject: [PATCH 13/15] simplify menubar logic --- primitives/src/menubar.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/primitives/src/menubar.rs b/primitives/src/menubar.rs index 38d8705..54103fd 100644 --- a/primitives/src/menubar.rs +++ b/primitives/src/menubar.rs @@ -239,7 +239,6 @@ pub fn MenubarTrigger(props: MenubarTriggerProps) -> Element { onfocus: move |_| ctx.set_focus(Some(menu_ctx.index)), onblur: move |_| { if (ctx.current_focus)() == Some(menu_ctx.index) && menu_ctx.current_focus.read().is_none() { - tracing::info!("Blur on menu {}", menu_ctx.index); ctx.set_focus(None); } }, @@ -260,14 +259,12 @@ pub struct MenubarContentProps { #[component] pub fn MenubarContent(props: MenubarContentProps) -> Element { - let ctx: MenubarContext = use_context(); let menu_ctx: MenubarMenuContext = use_context(); - let is_open = use_memo(move || (ctx.open_menu)() == Some(menu_ctx.index)); rsx! { div { role: "menu", - "data-state": if is_open() { "open" } else { "closed" }, + "data-state": if (menu_ctx.is_open)() { "open" } else { "closed" }, ..props.attributes, {props.children} } From 0cba6c363e6c06130deb2eda23c393814a9f12c0 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 11 Jun 2025 14:48:16 -0500 Subject: [PATCH 14/15] implement similar keyboard navigation logic for the dropdown menu component --- preview/src/components/menubar/mod.rs | 21 +-- primitives/src/dropdown_menu.rs | 196 +++++++++++--------------- primitives/src/menubar.rs | 19 ++- 3 files changed, 108 insertions(+), 128 deletions(-) diff --git a/preview/src/components/menubar/mod.rs b/preview/src/components/menubar/mod.rs index 7e0bbcb..2d7f3ff 100644 --- a/preview/src/components/menubar/mod.rs +++ b/preview/src/components/menubar/mod.rs @@ -4,8 +4,6 @@ use dioxus_primitives::menubar::{ }; #[component] pub(super) fn Demo() -> Element { - let mut selected_value = use_signal(String::new); - rsx! { document::Link { rel: "stylesheet", @@ -21,7 +19,7 @@ pub(super) fn Demo() -> Element { class: "menubar-item", value: "new".to_string(), on_select: move |value| { - selected_value.set(value); + tracing::info!("Selected value: {}", value); }, "New" } @@ -30,7 +28,7 @@ pub(super) fn Demo() -> Element { class: "menubar-item", value: "open".to_string(), on_select: move |value| { - selected_value.set(value); + tracing::info!("Selected value: {}", value); }, "Open" } @@ -39,7 +37,7 @@ pub(super) fn Demo() -> Element { class: "menubar-item", value: "save".to_string(), on_select: move |value| { - selected_value.set(value); + tracing::info!("Selected value: {}", value); }, "Save" } @@ -53,7 +51,7 @@ pub(super) fn Demo() -> Element { class: "menubar-item", value: "cut".to_string(), on_select: move |value| { - selected_value.set(value); + tracing::info!("Selected value: {}", value); }, "Cut" } @@ -62,7 +60,7 @@ pub(super) fn Demo() -> Element { class: "menubar-item", value: "copy".to_string(), on_select: move |value| { - selected_value.set(value); + tracing::info!("Selected value: {}", value); }, "Copy" } @@ -71,20 +69,13 @@ pub(super) fn Demo() -> Element { class: "menubar-item", value: "paste".to_string(), on_select: move |value| { - selected_value.set(value); + tracing::info!("Selected value: {}", value); }, "Paste" } } } } - div { class: "selected-value", - if selected_value().is_empty() { - "No selection" - } else { - "Selected: {selected_value()}" - } - } } } } diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index dba02ac..48504f3 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -20,33 +20,35 @@ impl DropdownMenuContext { if let Some(id) = id { self.recent_focus.set(id); } + if (self.open)() != id.is_some() { + (self.set_open)(id.is_some()); + } } fn focus_next(&mut self) { - if let Some(current_focus) = (self.current_focus)() { - let new_focus = (current_focus + 1) % (self.item_count)(); - self.current_focus.set(Some(new_focus)); - } + let focus = match (self.current_focus)() { + Some(current_focus) => (current_focus + 1) % self.item_count.cloned(), + None => 0, + }; + self.set_focus(Some(focus)); } fn focus_prev(&mut self) { - if let Some(current_focus) = (self.current_focus)() { - let new_focus = if current_focus == 0 { - (self.item_count)().saturating_sub(1) - } else { - current_focus - 1 - }; - self.current_focus.set(Some(new_focus)); - } + let item_count = self.item_count.cloned(); + let focus = match (self.current_focus)() { + Some(current_focus) if current_focus > 0 => current_focus - 1, + Some(_) | None => item_count.saturating_sub(1), + }; + self.set_focus(Some(focus)); } fn focus_first(&mut self) { - self.current_focus.set(Some(0)); + self.set_focus(Some(0)); } fn focus_last(&mut self) { let last = (self.item_count)().saturating_sub(1); - self.current_focus.set(Some(last)); + self.set_focus(Some(last)); } } @@ -73,47 +75,47 @@ pub struct DropdownMenuProps { pub fn DropdownMenu(props: DropdownMenuProps) -> Element { let (open, set_open) = use_controlled(props.open, props.default_open, props.on_open_change); + let disabled = props.disabled; let mut ctx = use_context_provider(|| DropdownMenuContext { open: open.into(), set_open, - disabled: props.disabled, + disabled, item_count: Signal::new(0), recent_focus: Signal::new(0), current_focus: Signal::new(None), }); - // Use a signal to track if we're in a click operation - let mut is_selecting = use_signal(|| false); - // Handle escape key to close the menu let handle_keydown = move |event: Event| { - if open() && event.key() == Key::Escape { - event.prevent_default(); - set_open.call(false); - ctx.set_focus(None); + if disabled() { + return; } + match event.key() { + Key::Enter => { + let new_open = !(ctx.open)(); + ctx.set_open.call(new_open); + } + Key::Escape => ctx.set_open.call(false), + Key::ArrowDown => { + ctx.focus_next(); + } + Key::ArrowUp => { + if open() { + ctx.focus_prev(); + } + } + Key::Home => ctx.focus_first(), + Key::End => ctx.focus_last(), + _ => return, + } + event.prevent_default(); }; rsx! { div { - tabindex: 0, - // We only close on focusout if we're not in a selection action - onfocusout: move |_| { - if open() && !is_selecting() && !is_selecting() { - set_open.call(false); - } - }, role: "menu", "data-state": if open() { "open" } else { "closed" }, "data-disabled": (props.disabled)(), - // Track mousedown events for selection - onmousedown: move |_| { - is_selecting.set(true); - }, - // Reset selection flag on mouseup - onmouseup: move |_| { - is_selecting.set(false); - }, onkeydown: handle_keydown, ..props.attributes, {props.children} @@ -130,7 +132,7 @@ pub struct DropdownMenuTriggerProps { #[component] pub fn DropdownMenuTrigger(props: DropdownMenuTriggerProps) -> Element { - let ctx: DropdownMenuContext = use_context(); + let mut ctx: DropdownMenuContext = use_context(); rsx! { button { @@ -145,6 +147,11 @@ pub fn DropdownMenuTrigger(props: DropdownMenuTriggerProps) -> Element { let new_open = !(ctx.open)(); ctx.set_open.call(new_open); }, + onblur: move |_| { + if ctx.current_focus.read().is_none() { + ctx.set_focus(None); + } + }, ..props.attributes, {props.children} @@ -161,43 +168,12 @@ pub struct DropdownMenuContentProps { #[component] pub fn DropdownMenuContent(props: DropdownMenuContentProps) -> Element { - let mut ctx: DropdownMenuContext = use_context(); - let open = ctx.open; - - // When menu opens, focus the first item - let is_open = open(); - use_effect(move || { - if is_open { - ctx.focus_first(); - } - }); + let ctx: DropdownMenuContext = use_context(); rsx! { div { role: "menu", - "data-state": if open() { "open" } else { "closed" }, - // "inert": !open(), - // Stop propagation to prevent unwanted interactions - onclick: move |e| { - e.stop_propagation(); - }, - onkeydown: move |event: Event| { - let mut prevent_default = true; - match event.key() { - Key::ArrowDown => ctx.focus_next(), - Key::ArrowUp => ctx.focus_prev(), - Key::Home => ctx.focus_first(), - Key::End => ctx.focus_last(), - Key::Escape => { - ctx.set_open.call(false); - ctx.set_focus(None); - } - _ => prevent_default = false, - } - if prevent_default { - event.prevent_default(); - } - }, + "data-state": if (ctx.open)() { "open" } else { "closed" }, ..props.attributes, {props.children} } @@ -227,66 +203,64 @@ pub fn DropdownMenuItem(props: DropdownMenuItemProps) -> Element { use_effect(move || { ctx.item_count += 1; }); - - // Cleanup when the component is unmounted use_effect_cleanup(move || { ctx.item_count -= 1; if (ctx.current_focus)() == Some((props.index)()) { - ctx.set_focus(None); + ctx.current_focus.set(None); } }); - let tab_index = use_memo(move || { - if (ctx.current_focus)() == Some((props.index)()) { - "0" - } else { - "-1" + let disabled = move || (ctx.disabled)() || (props.disabled)(); + let focused = move || (ctx.current_focus)() == Some((props.index)()); + + let mut item_ref: Signal>> = use_signal(|| None); + use_effect(move || { + let Some(item) = item_ref() else { + return; + }; + if focused() { + spawn(async move { + _ = item.set_focus(true).await; + }); } }); rsx! { div { role: "menuitem", - tabindex: tab_index, - "data-disabled": (ctx.disabled)() || (props.disabled)(), - - onclick: { - let value = (props.value)().clone(); - move |e: Event| { - e.stop_propagation(); - if !(ctx.disabled)() && !(props.disabled)() { - props.on_select.call(value.clone()); + "data-disabled": disabled(), + tabindex: if focused() { "0" } else { "-1" }, + + onclick: move |e: Event| { + e.stop_propagation(); + if !disabled() { + props.on_select.call((props.value)()); + ctx.set_open.call(false); + } + }, + + onkeydown: move |event: Event| { + if event.key() == Key::Enter { + if !disabled() { + props.on_select.call((props.value)()); ctx.set_open.call(false); } + event.prevent_default(); + event.stop_propagation(); } }, - onfocus: move |_| ctx.set_focus(Some((props.index)())), - - onkeydown: { - let value = (props.value)().clone(); - move |event: Event| { - let mut prevent_default = true; - match event.key() { - Key::Enter => { - if !(ctx.disabled)() && !(props.disabled)() { - props.on_select.call(value.clone()); - ctx.set_open.call(false); - } - } - Key::Escape => ctx.set_open.call(false), - Key::ArrowUp => ctx.focus_prev(), - Key::ArrowDown => ctx.focus_next(), - Key::Home => ctx.focus_first(), - Key::End => ctx.focus_last(), - _ => prevent_default = false, - } - if prevent_default { - event.prevent_default(); - } + onmounted: move |node| { + item_ref.set(Some(node.data())); + }, + + onblur: move |_| { + if focused() { + ctx.set_focus(None); } }, + ..props.attributes, {props.children} } diff --git a/primitives/src/menubar.rs b/primitives/src/menubar.rs index 54103fd..a8050d4 100644 --- a/primitives/src/menubar.rs +++ b/primitives/src/menubar.rs @@ -108,8 +108,7 @@ impl MenubarMenuContext { let item_count = self.item_count.cloned(); match &mut *current_focus { Some(current_focus) if *current_focus > 0 => *current_focus -= 1, - Some(current_focus) => *current_focus = item_count - 1, - None => *current_focus = Some(item_count - 1), + Some(_) | None => *current_focus = Some(item_count - 1), } } } @@ -333,9 +332,25 @@ pub fn MenubarItem(props: MenubarItemProps) -> Element { } } }, + + onkeydown: { + let value = props.value.clone(); + move |event: Event| { + if event.key() == Key::Enter { + if !disabled() { + props.on_select.call(value.clone()); + ctx.set_open_menu.call(None); + } + event.prevent_default(); + event.stop_propagation(); + } + } + }, + onmounted: move |node| { item_ref.set(Some(node.data())); }, + onblur: move |_| { if focused() { menu_ctx.current_focus.set(None); From 44749acfdc65a40cc3cea9a98ecff92702f26af2 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Wed, 11 Jun 2025 15:58:48 -0500 Subject: [PATCH 15/15] Keyboard navigation for the context menu component --- preview/src/components/context_menu/mod.rs | 16 +- .../src/components/dropdown_menu/style.css | 3 +- primitives/src/context_menu.rs | 182 ++++++++---------- primitives/src/dropdown_menu.rs | 2 +- primitives/src/menubar.rs | 2 +- 5 files changed, 101 insertions(+), 104 deletions(-) diff --git a/preview/src/components/context_menu/mod.rs b/preview/src/components/context_menu/mod.rs index 350b9fd..19f01ce 100644 --- a/preview/src/components/context_menu/mod.rs +++ b/preview/src/components/context_menu/mod.rs @@ -25,6 +25,9 @@ pub(super) fn Demo() -> Element { class: "context-menu-item", value: "edit".to_string(), index: 0usize, + on_select: move |value| { + tracing::info!("Selected item: {}", value); + }, "Edit" } ContextMenuItem { @@ -32,18 +35,27 @@ pub(super) fn Demo() -> Element { value: "undo".to_string(), index: 1usize, disabled: true, + on_select: move |value| { + tracing::info!("Selected item: {}", value); + }, "Undo" } ContextMenuItem { class: "context-menu-item", value: "duplicate".to_string(), - index: 1usize, + index: 2usize, + on_select: move |value| { + tracing::info!("Selected item: {}", value); + }, "Duplicate" } ContextMenuItem { class: "context-menu-item", value: "delete".to_string(), - index: 2usize, + index: 3usize, + on_select: move |value| { + tracing::info!("Selected item: {}", value); + }, "Delete" } } diff --git a/preview/src/components/dropdown_menu/style.css b/preview/src/components/dropdown_menu/style.css index da55a05..63307b9 100644 --- a/preview/src/components/dropdown_menu/style.css +++ b/preview/src/components/dropdown_menu/style.css @@ -42,11 +42,11 @@ padding: 0.25rem; box-shadow: inset 0 0 0 1px var(--dim-border-color); animation: slideIn 0.1s ease-out; + z-index: 1000; /* Animation properties */ opacity: 0; transform: translateY(-8px) scale(0.95); transition: all 0.2s ease; - pointer-events: none; } @media (prefers-color-scheme: dark) { @@ -60,7 +60,6 @@ .dropdown-menu-content[data-state="open"] { opacity: 1; transform: translateY(0) scale(1); - pointer-events: auto; } /* Closed state animations */ diff --git a/primitives/src/context_menu.rs b/primitives/src/context_menu.rs index 8834af7..c9e6e5b 100644 --- a/primitives/src/context_menu.rs +++ b/primitives/src/context_menu.rs @@ -19,62 +19,46 @@ struct ContextMenuCtx { impl ContextMenuCtx { fn set_focus(&mut self, index: Option) { - if let Some(idx) = index { - self.recent_focus.set(idx); - } self.current_focus.set(index); + if let Some(index) = index { + self.recent_focus.set(index); + } + if (self.open)() != index.is_some() { + (self.set_open)(index.is_some()); + } } fn focus_next(&mut self) { - let count = *self.item_count.read(); - if count == 0 { - return; - } - - let next = match *self.current_focus.read() { - Some(current) => (current + 1) % count, + let focus = match (self.current_focus)() { + Some(current_focus) => (current_focus + 1) % self.item_count.cloned(), None => 0, }; - self.set_focus(Some(next)); + self.set_focus(Some(focus)); } fn focus_prev(&mut self) { - let count = *self.item_count.read(); - if count == 0 { - return; - } - - let prev = match *self.current_focus.read() { - Some(current) => { - if current == 0 { - count - 1 - } else { - current - 1 - } - } - None => count - 1, + let item_count = self.item_count.cloned(); + let focus = match (self.current_focus)() { + Some(current_focus) if current_focus > 0 => current_focus - 1, + Some(_) | None => item_count.saturating_sub(1), }; - self.set_focus(Some(prev)); + self.set_focus(Some(focus)); } fn focus_first(&mut self) { - if *self.item_count.read() > 0 { - self.set_focus(Some(0)); - } + self.set_focus(Some(0)); } fn focus_last(&mut self) { - let count = *self.item_count.read(); - if count > 0 { - self.set_focus(Some(count - 1)); - } + let last = (self.item_count)().saturating_sub(1); + self.set_focus(Some(last)); } // Focus management helper - no actual focus restoration since we don't have NodeRef fn restore_trigger_focus(&mut self) { // In a real implementation with DOM access, we would focus the trigger element here // For now, we just reset the focus state - self.current_focus.set(None); + self.set_focus(None); } } @@ -125,22 +109,8 @@ pub fn ContextMenu(props: ContextMenuProps) -> Element { }; rsx! { - if open() { - div { - onclick: move |_| { - set_open.call(false); - ctx.restore_trigger_focus(); - }, - } - } div { tabindex: 0, // Make the menu container focusable - onfocusout: move |_| { - if open() { - set_open.call(false); - ctx.restore_trigger_focus(); - } - }, onkeydown: handle_keydown, "data-state": if open() { "open" } else { "closed" }, "data-disabled": (props.disabled)(), @@ -163,27 +133,18 @@ pub fn ContextMenuTrigger(props: ContextMenuTriggerProps) -> Element { let handle_context_menu = move |event: Event| { if !(ctx.disabled)() { - event.prevent_default(); ctx.position.set(( event.data().client_coordinates().x as i32, event.data().client_coordinates().y as i32, )); ctx.set_open.call(true); - } - }; - - // NEW: Handle left-click to close if already open - let handle_click = move |_| { - if (ctx.open)() { - ctx.set_open.call(false); - ctx.restore_trigger_focus(); + event.prevent_default(); } }; rsx! { div { oncontextmenu: handle_context_menu, - onclick: handle_click, aria_haspopup: "menu", aria_expanded: (ctx.open)(), ..props.attributes, @@ -203,17 +164,40 @@ pub struct ContextMenuContentProps { pub fn ContextMenuContent(props: ContextMenuContentProps) -> Element { let mut ctx: ContextMenuCtx = use_context(); let position = ctx.position; + let (x, y) = position(); - let style = use_memo(move || { - let (x, y) = position(); - format!("position: fixed; left: {x}px; top: {y}px;") - }); + let open = ctx.open; + + let onkeydown = move |event: Event| { + match event.key() { + Key::Escape => ctx.restore_trigger_focus(), + Key::ArrowDown => { + ctx.focus_next(); + } + Key::ArrowUp => { + if open() { + ctx.focus_prev(); + } + } + Key::Home => ctx.focus_first(), + Key::End => ctx.focus_last(), + _ => return, + } + event.prevent_default(); + }; - // When menu opens, focus the first item - let is_open = (ctx.open)(); + let mut menu_ref: Signal>> = use_signal(|| None); + let focused = move || open() && ctx.current_focus.read().is_none(); + // If the menu is open, but no item is focused, focus the div itself to capture events use_effect(move || { - if is_open { - ctx.focus_first(); + let Some(menu) = menu_ref() else { + return; + }; + if focused() { + spawn(async move { + // Focus the menu itself to capture keyboard events + _ = menu.set_focus(true).await; + }); } }); @@ -221,27 +205,18 @@ pub fn ContextMenuContent(props: ContextMenuContentProps) -> Element { div { role: "menu", aria_orientation: "vertical", - style: "{style}", - "data-state": if (ctx.open)() { "open" } else { "closed" }, - // "inert": !(ctx.open)(), - onclick: move |e| e.stop_propagation(), - onkeydown: move |event: Event| { - let mut prevent_default = true; - match event.key() { - Key::ArrowDown => ctx.focus_next(), - Key::ArrowUp => ctx.focus_prev(), - Key::Home => ctx.focus_first(), - Key::End => ctx.focus_last(), - Key::Escape => { - ctx.set_open.call(false); - ctx.restore_trigger_focus(); - } - _ => prevent_default = false, - } - if prevent_default { - event.prevent_default(); + position: "fixed", + left: "{x}px", + top: "{y}px", + tabindex: if focused() { "0" } else { "-1" }, + "data-state": if open() { "open" } else { "closed" }, + onkeydown, + onblur: move |_| { + if focused() { + ctx.restore_trigger_focus(); } }, + onmounted: move |evt| menu_ref.set(Some(evt.data())), ..props.attributes, {props.children} @@ -275,6 +250,7 @@ pub fn ContextMenuItem(props: ContextMenuItemProps) -> Element { let mut ctx: ContextMenuCtx = use_context(); let disabled = use_memo(move || (props.disabled)() || (ctx.disabled)()); + let focused = move || (ctx.current_focus)() == Some((props.index)()); // Register this item with the menu use_effect(move || { @@ -284,26 +260,31 @@ pub fn ContextMenuItem(props: ContextMenuItemProps) -> Element { // Cleanup when the component is unmounted use_effect_cleanup(move || { ctx.item_count -= 1; - if (ctx.current_focus)() == Some((props.index)()) { + if focused() { ctx.set_focus(None); } }); - // Determine if this item is currently focused - let tab_index = use_memo(move || { - if (ctx.current_focus)() == Some((props.index)()) { - "0" - } else { - "-1" + let mut item_ref: Signal>> = use_signal(|| None); + use_effect(move || { + let Some(item) = item_ref() else { + return; + }; + if focused() { + spawn(async move { + _ = item.set_focus(true).await; + }); } }); + // Determine if this item is currently focused + let tab_index = use_memo(move || if focused() { "0" } else { "-1" }); + let handle_click = { let value = (props.value)().clone(); move |_| { if !disabled() { props.on_select.call(value.clone()); - ctx.set_open.call(false); ctx.restore_trigger_focus(); } } @@ -313,13 +294,13 @@ pub fn ContextMenuItem(props: ContextMenuItemProps) -> Element { let value = (props.value)().clone(); move |event: Event| { // Check for Enter or Space key - if event.key() == Key::Enter || event.key().to_string() == " " { - event.prevent_default(); + if event.key() == Key::Enter || event.key() == Key::Character(" ".to_string()) { if !disabled() { props.on_select.call(value.clone()); - ctx.set_open.call(false); ctx.restore_trigger_focus(); } + event.prevent_default(); + event.stop_propagation(); } } }; @@ -328,9 +309,14 @@ pub fn ContextMenuItem(props: ContextMenuItemProps) -> Element { div { role: "menuitem", tabindex: tab_index, - onmousedown: handle_click, + onpointerdown: handle_click, onkeydown: handle_keydown, - onfocus: move |_| ctx.set_focus(Some((props.index)())), + onblur: move |_| { + if focused() { + ctx.set_focus(None); + } + }, + onmounted: move |evt| item_ref.set(Some(evt.data())), aria_disabled: disabled(), "data-disabled": disabled(), ..props.attributes, diff --git a/primitives/src/dropdown_menu.rs b/primitives/src/dropdown_menu.rs index 48504f3..442d2ba 100644 --- a/primitives/src/dropdown_menu.rs +++ b/primitives/src/dropdown_menu.rs @@ -240,7 +240,7 @@ pub fn DropdownMenuItem(props: DropdownMenuItemProps) -> Element { }, onkeydown: move |event: Event| { - if event.key() == Key::Enter { + if event.key() == Key::Enter || event.key() == Key::Character(" ".to_string()) { if !disabled() { props.on_select.call((props.value)()); ctx.set_open.call(false); diff --git a/primitives/src/menubar.rs b/primitives/src/menubar.rs index a8050d4..ca71dc3 100644 --- a/primitives/src/menubar.rs +++ b/primitives/src/menubar.rs @@ -336,7 +336,7 @@ pub fn MenubarItem(props: MenubarItemProps) -> Element { onkeydown: { let value = props.value.clone(); move |event: Event| { - if event.key() == Key::Enter { + if event.key() == Key::Enter || event.key() == Key::Character(" ".to_string()) { if !disabled() { props.on_select.call(value.clone()); ctx.set_open_menu.call(None);