Skip to content

Commit efe844e

Browse files
committed
Address review feedback
- Return early from the layer card key handler when the event came from a nested control. The card is a role="button" wrapper that called preventDefault on Enter, which cancelled the native activation of every action button inside it, the new palette button included. - Add an e2e test that opens the Style panel from the card by Enter and by Space. It fails without the guard above. - Match the Style panel aside by exact accessible name, so the collapsed rail ("Layer style (collapsed)") cannot satisfy the default substring match and mask the initial toHaveCount(0) assertion. - Note in the user guide that the palette button appears only when the built-in Style panel is enabled, since onOpenStylePanel is optional.
1 parent 80dbae8 commit efe844e

3 files changed

Lines changed: 32 additions & 2 deletions

File tree

apps/geolibre-desktop/src/components/panels/LayerPanel.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3210,6 +3210,10 @@ export function LayerPanel({
32103210
aria-pressed={selectedLayerIds.has(layer.id)}
32113211
onClick={(e) => handleLayerSelection(e, layer.id)}
32123212
onKeyDown={(e) => {
3213+
// Only act on the card itself: preventDefault here would
3214+
// otherwise cancel the Enter activation of the action
3215+
// buttons nested inside it.
3216+
if (e.target !== e.currentTarget) return;
32133217
if (e.key === "Enter" || e.key === " ") {
32143218
e.preventDefault();
32153219
setSelectedLayerIds(new Set([layer.id]));

docs/user-guide/layers.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ The **Layers panel** on the left lists every layer in the project, from the topm
1414

1515
Each layer exposes a set of actions:
1616

17-
- **Open Style panel**: use the palette button on the layer card to select the layer and open its styling controls.
17+
- **Open Style panel**: when the built-in Style panel is enabled, use the palette button on the layer card to select the layer and open its styling controls.
1818
- **Zoom to layer**: fit the map to the layer's extent (for layers whose bounds are known).
1919
- **Identify features**: click features on the map to see their attributes in a popup. On a raster layer this reads the pixel value instead, and on a multiband raster it also builds a [spectral profile](styling.md#spectral-profile).
2020
- **Labels**: toggle text labels for vector layers that have a label field.

e2e/layer-panel.spec.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,11 +117,37 @@ test("opens the selected layer in the Style panel from its card", async ({ page
117117
await dropGeoJson(page, "second", FIXTURE_TEXT);
118118
await expect(layerRow(page, "second")).toBeVisible();
119119

120-
const stylePanel = page.getByRole("complementary", { name: "Layer style" });
120+
// Exact, so the collapsed rail ("Layer style (collapsed)") cannot satisfy the
121+
// default substring match.
122+
const stylePanel = page.getByRole("complementary", { name: "Layer style", exact: true });
121123
await expect(stylePanel).toHaveCount(0);
122124

123125
await layerRow(page, "first").getByRole("button", { name: "Open Style panel" }).click();
124126

125127
await expect(stylePanel).toBeVisible();
126128
await expect(stylePanel.getByText("Style - first", { exact: true })).toBeVisible();
127129
});
130+
131+
test("opens the Style panel from the layer card by keyboard", async ({ page }) => {
132+
await page.setViewportSize({ width: 768, height: 720 });
133+
await waitForMap(page);
134+
await dropGeoJson(page, "first", FIXTURE_TEXT);
135+
await expect(layerRow(page, "first")).toBeVisible();
136+
await dropGeoJson(page, "second", FIXTURE_TEXT);
137+
await expect(layerRow(page, "second")).toBeVisible();
138+
139+
const stylePanel = page.getByRole("complementary", { name: "Layer style", exact: true });
140+
const styleButton = (name: string) =>
141+
layerRow(page, name).getByRole("button", { name: "Open Style panel" });
142+
143+
// The card is a role="button" wrapper; its key handler must not swallow the
144+
// activation of the action buttons nested inside it.
145+
await styleButton("first").focus();
146+
await page.keyboard.press("Enter");
147+
await expect(stylePanel).toBeVisible();
148+
await expect(stylePanel.getByText("Style - first", { exact: true })).toBeVisible();
149+
150+
await styleButton("second").focus();
151+
await page.keyboard.press("Space");
152+
await expect(stylePanel.getByText("Style - second", { exact: true })).toBeVisible();
153+
});

0 commit comments

Comments
 (0)