Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Open unlimited polygon does not restrict crossing sides. #1896

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/chilled-pianos-cheat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Fixing intersecting polygon sides issues for unlimited sided polygon.
10 changes: 8 additions & 2 deletions packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export const MafsGraph = (props: MafsGraphProps) => {
const uniqueId = React.useId();
const descriptionId = `interactive-graph-description-${uniqueId}`;
const interactiveElementsDescriptionId = `interactive-graph-interactive-elements-description-${uniqueId}`;
const unlimitedGraphKeyboardPromptId = `unlimited-graph-keyboard-prompt-${uniqueId}`;
const graphRef = React.useRef<HTMLElement>(null);
const {analytics} = useDependencies();

Expand Down Expand Up @@ -173,6 +174,8 @@ export const MafsGraph = (props: MafsGraphProps) => {
fullGraphAriaDescription && descriptionId,
interactiveElementsDescription &&
interactiveElementsDescriptionId,
isUnlimitedGraphState(state) &&
"unlimited-graph-keyboard-prompt",
)}
ref={graphRef}
tabIndex={0}
Expand Down Expand Up @@ -301,6 +304,9 @@ export const MafsGraph = (props: MafsGraphProps) => {
{interactionPrompt && (
<View
style={{
display: interactionPrompt
? undefined
: "hidden",
textAlign: "center",
backgroundColor: "white",
border: "1px solid #21242C52",
Expand All @@ -314,7 +320,7 @@ export const MafsGraph = (props: MafsGraphProps) => {
transform: "translateY(-50%)",
}}
>
<LabelMedium>
<LabelMedium id={unlimitedGraphKeyboardPromptId}>
{strings.graphKeyboardPrompt}
</LabelMedium>
</View>
Expand Down Expand Up @@ -440,7 +446,7 @@ const renderPolygonGraphControls = (props: {
width: "100%",
marginLeft: "20px",
}}
tabIndex={0}
tabIndex={coords.length < 3 ? -1 : 0}
onClick={() => {
props.dispatch(actions.polygon.closePolygon());
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,29 @@ describe("movePoint on a polygon graph", () => {
expect(updated.coords[0]).toEqual([0, 0]);
});

it("does not reject intersecting sides if the polygon is unlimited and it's open", () => {
const state: InteractiveGraphState = {
...basePolygonGraphState,
numSides: "unlimited",
closedPolygon: false,
snapTo: "grid",
coords: [
[0, 0],
[0, 2],
[2, 2],
[2, 0],
],
};

const updated = interactiveGraphReducer(
state,
actions.polygon.movePoint(0, [1, 3]),
);

invariant(updated.type === "polygon");
expect(updated.coords[0]).toEqual([1, 3]);
});

it("does not snap to grid when snapTo is angles using moveAll", () => {
const state: InteractiveGraphState = {
...basePolygonGraphState,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,13 @@ function doMovePoint(
newValue: newValue,
});

// Reject the move if it would cause the sides of the polygon to cross
if (polygonSidesIntersect(newCoords)) {
// Boolean value to tract whether we can let the polygon sides interact.
// They can interact if it's an unlimited polygon that is open.
const polygonSidesCanIntersect =
state.numSides === "unlimited" && !state.closedPolygon;

// Reject the move if it would cause the sides of the polygon to cross.
if (!polygonSidesCanIntersect && polygonSidesIntersect(newCoords)) {
return state;
}

Expand Down