From c0f76dfff80039bd498859b41d02e4b44cc4d426 Mon Sep 17 00:00:00 2001 From: Jamal Ali Date: Sun, 6 Sep 2026 01:12:24 +0400 Subject: [PATCH] fix(layer-list): title visibility button with the action it performs (#2135) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fixes #1675 ## Problem In the layer list, the show/hide button's `title` tooltip named the layer's current visibility instead of what a click does. A visible layer's button read `show` while clicking it hides the layer, and a hidden layer's button read `hide` while clicking it shows the layer — exactly reversed. `LayerListItem.tsx` derived a single value from the layer state: ```ts const visibilityAction = visibility === "visible" ? "show" : "hide"; ``` and passed it to `IconAction` as `action`, which `IconAction` used for three different things: choosing the icon, building the `--show`/`--hide` CSS modifier, and rendering `title={this.props.action}`. The first two are correct as a state indicator; only the tooltip needs the opposite verb. ## Change - `IconAction` gets an optional `title` prop and falls back to `action` when it is not given, so the delete and duplicate buttons are unchanged. - `LayerListItem` keeps `visibilityAction` (icon + `maputnik-layer-list-icon-action__visibility--hide` CSS modifier, which `_layer.scss` relies on to keep the button visible for hidden layers) and adds `visibilityTitle` for the inverted tooltip. No icon, class name or click behaviour changes. ## Test New regression test in `e2e/layers-list.spec.ts`: it asserts the button is titled `hide` while the layer is visible, clicks it, and asserts it is titled `show` once the layer is hidden. Against the unpatched component the first assertion fails with `Expected: "hide"` / `Received: "show"`, reproducing the reported behaviour. `npm run lint`, `npx tsc --noEmit`, `npx vitest run` (50 tests) and `npx playwright test e2e/layers-list.spec.ts` (28 tests) all pass. ## Note The tooltips in this component (`delete`, `duplicate`, `show`, `hide`) are plain English literals and are not run through `t()` today, so this change keeps them as-is rather than introducing translation keys for one button. Also worth flagging separately: these buttons carry `aria-hidden="true"`, so the `title` never reaches assistive technology at all — out of scope here, but it means the label is a mouse-hover tooltip only. --- e2e/layers-list.spec.ts | 10 +++++++++- src/components/LayerListItem.tsx | 12 ++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/e2e/layers-list.spec.ts b/e2e/layers-list.spec.ts index 56c3529b..85cdd4a1 100644 --- a/e2e/layers-list.spec.ts +++ b/e2e/layers-list.spec.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, test } from "./utils/fixtures"; +import { beforeEach, describe, expect, test } from "./utils/fixtures"; import { MaputnikDriver } from "./maputnik-driver"; describe("layers list", () => { @@ -47,6 +47,14 @@ describe("layers list", () => { }); }); + test("should title the visibility button with the action it performs", async () => { + const key = "layer-list-item:" + id + ":toggle-visibility"; + // A visible layer is about to be hidden, and a hidden one to be shown. + await expect(get.elementByTestId(key)).toHaveAttribute("title", "hide"); + await when.click(key); + await expect(get.elementByTestId(key)).toHaveAttribute("title", "show"); + }); + describe("when clicking hide", () => { beforeEach(async () => { await when.click("layer-list-item:" + id + ":toggle-visibility"); diff --git a/src/components/LayerListItem.tsx b/src/components/LayerListItem.tsx index 94fd1f95..f4d9d475 100644 --- a/src/components/LayerListItem.tsx +++ b/src/components/LayerListItem.tsx @@ -32,6 +32,8 @@ const DraggableLabel: React.FC = (props) => { type IconActionProps = { action: string + /** Tooltip text, for buttons whose action reads differently from their icon. */ + title?: string onClick(...args: unknown[]): unknown wdKey?: string classBlockName?: string @@ -62,7 +64,7 @@ class IconAction extends React.Component { return