Skip to content

Commit

Permalink
Update Polygon graphs to have weighted lines only after focus from ke…
Browse files Browse the repository at this point in the history
…yboard (#1455)

## Summary:
Previously, Polygon had its shape weighted upon hover and after clicking in the shape. This is not how circle is currently working. To bring them into alignment, this changes Polygon to become weighted only when the shape is tabbed to by the keyboard or if the shape is moved via keyboard after the user clicks on it. 

### Before
https://github.com/user-attachments/assets/16f3aef8-3312-4e0c-8815-542ad6ea7c98

### After
https://github.com/user-attachments/assets/b6058ae6-b31e-4942-ae24-9ae2853bfcc3

Issue: LEMS-2200

## Test plan:
- Checkout the associated branch
- Run `yarn dev` 
- On the gallery page, turn on the flag for polygon and circle
- Confirm Polygon does not have weighted lines on hover or when dragging. After clicking the shape, confirm the shape becomes weighted when you move it with the arrow keys. Confirm the lines return to normal after clicking outside of the shape.
- Confirm this functionality matches Circle graphs

Author: Myranae

Reviewers: benchristel, nishasy, mark-fitzgerald, nicolecomputer, SonicScrewdriver

Required Reviewers:

Approved By: benchristel

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1455
  • Loading branch information
Myranae authored Aug 1, 2024
1 parent ae8f208 commit 8b02682
Show file tree
Hide file tree
Showing 4 changed files with 108 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .changeset/cyan-seals-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Update Polygon graphs to have weighted lines only on keyboard focus
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ interface PolygonAngleProps {
centerPoint: vec.Vector2;
endPoints: [vec.Vector2, vec.Vector2];
polygonLines: readonly CollinearTuple[];
active: boolean;
range: [Interval, Interval];
showAngles: boolean;
snapTo: "grid" | "angles" | "sides";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import {render} from "@testing-library/react";
import {userEvent as userEventLib} from "@testing-library/user-event";
import {Mafs, Polygon} from "mafs";
import React from "react";

import {hasFocusVisible} from "./polygon";

import type {UserEvent} from "@testing-library/user-event";

describe("hasFocusVisible", () => {
let userEvent: UserEvent;
beforeEach(() => {
userEvent = userEventLib.setup({
advanceTimers: jest.advanceTimersByTime,
});
});

it("returns true when polygon is focused", async () => {
const ref = React.createRef<SVGPolygonElement>();
render(
<Mafs width={200} height={200}>
<Polygon
points={[
[0, 0],
[0, 2],
[2, 2],
[2, 0],
]}
svgPolygonProps={{
ref,
tabIndex: 0,
}}
/>
</Mafs>,
);
const polygon = ref.current;
if (polygon) {
await userEvent.tab();
await userEvent.tab();
}

expect(polygon).toBeInTheDocument();
expect(polygon).toHaveFocus();
expect(hasFocusVisible(polygon)).toBe(true);
});

it("returns false when polygon is not focused", async () => {
const ref = React.createRef<SVGPolygonElement>();
render(
<Mafs width={200} height={200}>
<Polygon
points={[
[0, 0],
[0, 2],
[2, 2],
[2, 0],
]}
svgPolygonProps={{
ref,
tabIndex: 0,
}}
/>
</Mafs>,
);
const polygon = ref.current;
if (polygon) {
await userEvent.tab();
}

expect(polygon).toBeInTheDocument();
expect(polygon).not.toHaveFocus();
expect(hasFocusVisible(polygon)).toBe(false);
});
});
35 changes: 29 additions & 6 deletions packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ import type {CollinearTuple} from "@khanacademy/perseus";
type Props = MafsGraphProps<PolygonGraphState>;

export const PolygonGraph = (props: Props) => {
const [focused, setFocused] = React.useState(false);
const [hovered, setHovered] = React.useState(false);
// This is more so required for the re-rendering that occurs when state
// updates; specifically with regard to line weighting and polygon focus.
const [focusVisible, setFocusVisible] = React.useState(false);

const {dispatch} = props;
const {coords, showAngles, showSides, range, snapStep, snapTo} =
Expand All @@ -43,7 +45,6 @@ export const PolygonGraph = (props: Props) => {
["angles", "sides"].includes(snapToValue) ? p : snap(snapStep, p),
});

const active = hovered || focused || dragging;
const lastMoveTime = React.useRef<number>(0);

const lines = getLines(points);
Expand All @@ -54,7 +55,7 @@ export const PolygonGraph = (props: Props) => {
points={[...points]}
color="var(--movable-line-stroke-color)"
svgPolygonProps={{
strokeWidth: active
strokeWidth: focusVisible
? "var(--movable-line-stroke-weight-active)"
: "var(--movable-line-stroke-weight)",
style: {fill: "transparent"},
Expand All @@ -71,7 +72,6 @@ export const PolygonGraph = (props: Props) => {
key={"angle-" + i}
centerPoint={point}
endPoints={[pt1, pt2]}
active={active}
range={range}
polygonLines={lines}
showAngles={!!showAngles}
Expand Down Expand Up @@ -109,10 +109,19 @@ export const PolygonGraph = (props: Props) => {
cursor: dragging ? "grabbing" : "grab",
fill: hovered ? "var(--mafs-blue)" : "transparent",
},
onFocus: () => setFocused(true),
onBlur: () => setFocused(false),
onMouseEnter: () => setHovered(true),
onMouseLeave: () => setHovered(false),
// Required to remove line weighting when user clicks away
// from the focused polygon
onKeyDownCapture: () => {
setFocusVisible(hasFocusVisible(ref.current));
},
// Required for lines to darken on focus
onFocus: () =>
setFocusVisible(hasFocusVisible(ref.current)),
// Required for line weighting to update on blur. Without this,
// the user has to hover over the shape for it to update
onBlur: () => setFocusVisible(hasFocusVisible(ref.current)),
className: "movable-polygon",
}}
/>
Expand Down Expand Up @@ -143,3 +152,17 @@ function getLines(points: readonly vec.Vector2[]): CollinearTuple[] {
return [point, next];
});
}

export const hasFocusVisible = (
element: Element | null | undefined,
): boolean => {
const matches = (selector: string) => element?.matches(selector) ?? false;
try {
return matches(":focus-visible");
} catch (e) {
// jsdom doesn't support :focus-visible
// (see https://github.com/jsdom/jsdom/issues/3426),
// so the call to matches(":focus-visible") will fail in tests.
return matches(":focus");
}
};

0 comments on commit 8b02682

Please sign in to comment.