mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-10-07 15:58:03 +08:00
fix(settings): show ACP-disabled tooltip on desktop (drop pointer-events-none) (#1499)
* fix(settings): show ACP-disabled tooltip on desktop (drop pointer-events-none)
When an ACP agent is active, the desktop settings sidebar greys out the
LLM/Condenser/Verification items and wraps them in a hover StyledTooltip
("Disabled while {agent} is active"). The tooltip never opened, so users
saw a greyed item with no explanation (reported on macOS desktop).
Root cause: the disabled link also got `pointer-events-none`, but
StyledTooltip is HeroUI's pointer-driven Tooltip — with pointer events
suppressed, onPointerEnter never fires and the tooltip can't open.
pointer-events-none wasn't needed for correctness either: onClick already
preventDefaults navigation, and tabIndex=-1 + aria-disabled cover
keyboard/AT.
Fix: keep the item visually greyed (opacity-50) but only apply
pointer-events-none when there's no disabledReason tooltip to show.
Fixes #1498
Co-authored-by: smolpaws <engel@enyst.org>
* chore: Remove PR-only artifacts
* Address ACP tooltip review comment
* Restore sidebar disabled aria comment
---------
Co-authored-by: smolpaws <engel@enyst.org>
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
committed by
GitHub
co-authored by
smolpaws
openhands
parent
3f52df2e39
commit
1294b08944
Binary file not shown.
|
Before Width: | Height: | Size: 16 KiB |
Binary file not shown.
|
Before Width: | Height: | Size: 167 KiB |
Binary file not shown.
|
Before Width: | Height: | Size: 122 KiB |
@@ -1,5 +1,5 @@
|
||||
import type { ReactNode } from "react";
|
||||
import { render, screen, waitFor, within } from "@testing-library/react";
|
||||
import { render, screen, within } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { MemoryRouter } from "react-router";
|
||||
@@ -149,6 +149,37 @@ describe("SettingsNavigation", () => {
|
||||
expect(condenserLink).toHaveAttribute("aria-disabled", "true");
|
||||
});
|
||||
|
||||
it("keeps pointer events on disabled-by-ACP items so the hover tooltip can open", () => {
|
||||
// Regression for the missing "Disabled while {agent} is active" tooltip
|
||||
// on desktop: the disabled link used to also get ``pointer-events-none``,
|
||||
// which stops HeroUI's pointer-driven Tooltip from ever firing — so the
|
||||
// explanation never reached the user (reported on macOS desktop). A
|
||||
// disabled item that HAS a reason must stay greyed (opacity-50) but keep
|
||||
// receiving pointer events.
|
||||
renderSettingsNavigation(
|
||||
<SettingsNavigation
|
||||
isMobileMenuOpen={false}
|
||||
onCloseMobileMenu={vi.fn()}
|
||||
navigationItems={[
|
||||
{
|
||||
type: "item",
|
||||
item: llmItem,
|
||||
disabled: true,
|
||||
disabledAgentName: "Claude Code",
|
||||
},
|
||||
]}
|
||||
/>,
|
||||
);
|
||||
|
||||
const desktopNav = screen.getByTestId("settings-navbar-desktop");
|
||||
const llmLink = within(desktopNav).getByTestId(
|
||||
"sidebar-settings-/settings/llm",
|
||||
);
|
||||
expect(llmLink).toHaveAttribute("aria-disabled", "true");
|
||||
expect(llmLink.className).toContain("opacity-50");
|
||||
expect(llmLink.className).not.toContain("pointer-events-none");
|
||||
});
|
||||
|
||||
it("leaves enabled items clickable in the desktop sidebar", () => {
|
||||
renderSettingsNavigation(
|
||||
<SettingsNavigation
|
||||
|
||||
@@ -73,10 +73,10 @@ export function SidebarNavLink({
|
||||
data-testid={testId}
|
||||
tabIndex={disabled ? -1 : 0}
|
||||
aria-label={collapsed ? label : undefined}
|
||||
// Announce the disabled state to assistive tech. The visual disable
|
||||
// (opacity + pointer-events) plus tabIndex=-1 + preventDefault gives
|
||||
// sighted/keyboard users the right behaviour already; this closes
|
||||
// the screen-reader gap so the link doesn't sound "actionable."
|
||||
// Announce the disabled state to assistive tech. The visual disabled
|
||||
// styling plus tabIndex=-1 + preventDefault gives sighted/keyboard users
|
||||
// the right behaviour already; this closes the screen-reader gap so the
|
||||
// link doesn't sound "actionable."
|
||||
aria-disabled={disabled || undefined}
|
||||
onClick={(e) => {
|
||||
if (disabled) {
|
||||
@@ -89,7 +89,9 @@ export function SidebarNavLink({
|
||||
(active
|
||||
? SIDEBAR_ROW_INTERACTIVE_CLASS.active
|
||||
: SIDEBAR_ROW_INTERACTIVE_CLASS.idle),
|
||||
disabled && "pointer-events-none opacity-50",
|
||||
disabled && "opacity-50",
|
||||
// HeroUI Tooltip is pointer-driven, so keep hover events for explanations.
|
||||
disabled && !disabledReason && "pointer-events-none",
|
||||
)}
|
||||
>
|
||||
{icon ? (
|
||||
|
||||
Reference in New Issue
Block a user