Skip to content

leptos-shadcn-context-menu's ContextMenuTrigger leaks document click listeners via closure.forget() #6

Description

@Boscop

@petehanssens Thanks for making this crate, it seems very useful!

I noticed that leptos-shadcn-context-menu's ContextMenuTrigger leaks document click listeners via closure.forget().

Can you please fix this? 🙂🙏

Also, it would be great if the lib would allow only one context menu to be open at a time (auto-closing older ones when opening a new one).

My current workaround (specific to my use case, not fully general):

//! Custom context menu components with proper cleanup.
//!
//! These wrap leptos-shadcn-context-menu but fix the memory leak in ContextMenuTrigger
//! and add global menu coordination to ensure only one menu is open at a time.
//!
//! The library's ContextMenuTrigger leaks document click listeners via closure.forget().
//! Our solution uses a single global document click listener
//! that clears OpenDeckContextMenu, and each MyContextMenuContent reacts to close itself.

use leptos::prelude::*;

/// Unique identifier for a deck context menu (large view vs small view deck)
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
pub enum DeckContextMenuId {
	/// Large deck view at top (deck_id)
	Large(usize),
	/// Small deck view in deck panel (deck_id)
	Small(usize),
}

/// Global signal tracking which deck context menu is currently open (if any)
/// Setting this to Some(id) will close any other open deck context menu
#[derive(Clone, Copy)]
pub struct OpenDeckContextMenu(pub RwSignal<Option<DeckContextMenuId>>);

/// Custom ContextMenuTrigger that properly cleans up event listeners.
/// Instead of per-trigger listeners, a single global document click listener
/// in MainLayout clears OpenDeckContextMenu, which triggers MyContextMenuContent to close.
#[component]
pub fn MyContextMenuTrigger(
	#[prop(into, optional)] class: MaybeProp<String>,
	#[prop(optional)] children: Option<Children>,
) -> impl IntoView {
	let open = expect_context::<RwSignal<bool>>();
	let position = expect_context::<RwSignal<(i32, i32)>>();

	let handle_context_menu = move |e: web_sys::MouseEvent| {
		e.prevent_default();
		position.set((e.client_x(), e.client_y()));
		open.set(true);
	};

	// No per-trigger document listener needed - MainLayout has a single global one
	// that clears OpenDeckContextMenu, causing MyContextMenuContent to close

	view! {
		<div class=class on:contextmenu=handle_context_menu>
			{children.map(|c| c())}
		</div>
	}
}

#[component]
pub fn MyContextMenuContent(
	#[prop(into, optional)] class: MaybeProp<String>,
	/// Unique ID for this context menu - used to close other menus when this one opens
	#[prop(into)]
	menu_id: DeckContextMenuId,
	#[prop(optional)] children: Option<Children>,
) -> impl IntoView {
	let open = expect_context::<RwSignal<bool>>();
	let position = expect_context::<RwSignal<(i32, i32)>>();
	let OpenDeckContextMenu(global_open_menu) =
		use_context::<OpenDeckContextMenu>().expect("OpenDeckContextMenu context");

	// When this menu opens, set it as the globally open menu (closes others)
	Effect::new(move |_| {
		if open.get() {
			global_open_menu.set(Some(menu_id));
		}
	});

	// When global changes to a different menu, close this one
	Effect::new(move |_| {
		let global_id = global_open_menu.get();
		if open.get_untracked() && global_id != Some(menu_id) {
			open.set(false);
		}
	});

	let handle_click = move |e: web_sys::MouseEvent| {
		e.stop_propagation();
	};

	let content_style = move || {
		let (x, y) = position.get();
		format!("position: fixed; left: {}px; top: {}px; z-index: 50;", x, y)
	};

	view! {
		<div
			class=class
			class:hidden=move || !open.get()
			style=content_style
			on:click=handle_click
			role="menu"
			aria-orientation="vertical"
		>
			{children.map(|c| c())}
		</div>
	}
}

#[component]
pub fn MyContextMenuActionItem(
	#[prop(into, optional)] class: MaybeProp<String>,
	#[prop(into)] disabled: Signal<bool>,
	#[prop(into)] on_select: Callback<()>,
	#[prop(optional)] children: Option<Children>,
) -> impl IntoView {
	let open = expect_context::<RwSignal<bool>>();

	let item_class = move || {
		let base_class = "relative flex cursor-pointer select-none items-center gap-2 rounded-md px-3 py-2 text-sm font-medium outline-none transition-colors duration-150 hover:bg-gray-100 dark:hover:bg-gray-700 focus:bg-gray-100 dark:focus:bg-gray-700 active:bg-gray-200 dark:active:bg-gray-600 data-[disabled=true]:pointer-events-none data-[disabled=true]:opacity-50";
		format!("{} {}", base_class, class.get().unwrap_or_default())
	};

	let on_click = move |e: web_sys::MouseEvent| {
		if disabled.get() {
			return;
		}
		e.stop_propagation();
		open.set(false);
		on_select.run(());
	};

	view! {
		<div
			class=item_class
			on:click=on_click
			role="menuitem"
			aria-disabled=move || disabled.get()
			attr:data-disabled=move || if disabled.get() { "true" } else { "false" }
		>
			{children.map(|c| c())}
		</div>
	}
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions