Skip to content

Commit

Permalink
Merge pull request #2335 from concord-consortium/185538875-label-poly…
Browse files Browse the repository at this point in the history
…gons

Geometry label polygon dialog
  • Loading branch information
bgoldowsky authored Jul 10, 2024
2 parents 46ccb48 + ba14f80 commit 270ef60
Show file tree
Hide file tree
Showing 22 changed files with 360 additions and 115 deletions.
6 changes: 3 additions & 3 deletions cypress/e2e/functional/document_tests/copy_doc_test_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ context('Copy Document', () => {
clueCanvas.addTile('geometry');
geometryTile.getGeometryTile().last().click();
clueCanvas.clickToolbarButton('geometry', 'point');
geometryTile.addPointToGraph(5, 5);
geometryTile.addPointToGraph(10, 5);
geometryTile.addPointToGraph(10, 10);
geometryTile.clickGraphPosition(5, 5);
geometryTile.clickGraphPosition(10, 5);
geometryTile.clickGraphPosition(10, 10);
geometryTile.getGraphPoint().should('have.length', 4);

cy.log('Add drawing tile');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ function setupTest(studentIndex) {
cy.get('.spacer').click();
geometryToolTile.getGeometryTile().last().click();
clueCanvas.clickToolbarButton('geometry', 'point');
geometryToolTile.addPointToGraph(5, 5);
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.addPointToGraph(10, 10);
geometryToolTile.clickGraphPosition(5, 5);
geometryToolTile.clickGraphPosition(10, 5);
geometryToolTile.clickGraphPosition(10, 10);
geometryToolTile.getGraphPoint().should('have.length', 4); // including phantom point
clueCanvas.addTile("drawing");
drawToolTile.getDrawToolRectangle().click();
Expand Down
6 changes: 3 additions & 3 deletions cypress/e2e/functional/document_tests/tiles_copy_test_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ context('Test copy tiles from one document to other document', function () {
cy.get('.spacer').click();
geometryToolTile.getGeometryTile().last().click();
clueCanvas.clickToolbarButton('geometry', 'point');
geometryToolTile.addPointToGraph(5, 5);
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.addPointToGraph(10, 10);
geometryToolTile.clickGraphPosition(5, 5);
geometryToolTile.clickGraphPosition(10, 5);
geometryToolTile.clickGraphPosition(10, 10);
geometryToolTile.getGraphPoint().should('have.length', 4);

cy.log('Add drawing tile');
Expand Down
8 changes: 4 additions & 4 deletions cypress/e2e/functional/tile_tests/arrow_annotation_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,10 @@ context('Arrow Annotations (Sparrows)', function () {

aa.clickArrowToolbarButton(); // sparrow mode off
geometryToolTile.getGeometryTile().click(); // select tile
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.addPointToGraph(15, 10);
geometryToolTile.addPointToGraph(20, 5);
geometryToolTile.addPointToGraph(10, 5); // close polygon
geometryToolTile.clickGraphPosition(10, 5);
geometryToolTile.clickGraphPosition(15, 10);
geometryToolTile.clickGraphPosition(20, 5);
geometryToolTile.clickGraphPosition(10, 5); // close polygon

aa.clickArrowToolbarButton(); // sparrow mode on
// 3 points + 3 segments + 1 polygon = 7
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,10 @@ context('Geometry Table Integration', function () {
cy.log('normal geometry interactions');
cy.log('will add a polygon directly onto the geometry');
geometryToolTile.getGeometryTile().click();
geometryToolTile.addPointToGraph(10, 10); //not sure why this isn't appearing
geometryToolTile.addPointToGraph(10, 10);
geometryToolTile.addPointToGraph(15, 10);
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.clickGraphPosition(10, 10); //not sure why this isn't appearing
geometryToolTile.clickGraphPosition(10, 10);
geometryToolTile.clickGraphPosition(15, 10);
geometryToolTile.clickGraphPosition(10, 5);
geometryToolTile.getGraphPoint().last().click({ force: true }).click({ force: true });

cy.log('will add an angle to a point created from a table');
Expand Down
41 changes: 30 additions & 11 deletions cypress/e2e/functional/tile_tests/geometry_tool_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ context('Geometry Tool', function () {
cy.log("add a point to the origin");
clueCanvas.addTile('geometry');
clueCanvas.clickToolbarButton('geometry', 'point');
geometryToolTile.addPointToGraph(0, 0);
geometryToolTile.clickGraphPosition(0, 0);
geometryToolTile.getGraphPointCoordinates().should('exist');

cy.log("add points to a geometry");
Expand All @@ -37,9 +37,9 @@ context('Geometry Tool', function () {
cy.get('.spacer').click();
geometryToolTile.getGeometryTile().last().click();
clueCanvas.clickToolbarButton('geometry', 'point');
geometryToolTile.addPointToGraph(5, 5);
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.addPointToGraph(10, 10);
geometryToolTile.clickGraphPosition(5, 5);
geometryToolTile.clickGraphPosition(10, 5);
geometryToolTile.clickGraphPosition(10, 10);

cy.log("copy a point to the clipboard");
let clipSpy;
Expand Down Expand Up @@ -117,8 +117,8 @@ context('Geometry Tool', function () {
clueCanvas.toolbarButtonIsSelected('geometry', 'point');
geometryToolTile.getGraph().trigger('mousemove');
geometryToolTile.getGraphPoint().should("have.length", 1); // phantom point
geometryToolTile.addPointToGraph(1, 1);
geometryToolTile.addPointToGraph(2, 2);
geometryToolTile.clickGraphPosition(1, 1);
geometryToolTile.clickGraphPosition(2, 2);
geometryToolTile.getGraphPoint().should("have.length", 3);

// Duplicate point
Expand All @@ -140,7 +140,7 @@ context('Geometry Tool', function () {
geometryToolTile.getGraphPoint().should("have.length", 2); // no phantom point

// Clicking background should NOT create a point.
geometryToolTile.addPointToGraph(3, 3);
geometryToolTile.clickGraphPosition(3, 3);
geometryToolTile.getGraphPoint().should("have.length", 2); // same as before

geometryToolTile.getSelectedGraphPoint().should("have.length", 0);
Expand Down Expand Up @@ -174,13 +174,13 @@ context('Geometry Tool', function () {
clueCanvas.toolbarButtonIsSelected('geometry', 'polygon');
geometryToolTile.getGraph().trigger('mousemove');
geometryToolTile.getGraphPoint().should("have.length", 1); // phantom point
geometryToolTile.addPointToGraph(5, 5);
geometryToolTile.clickGraphPosition(5, 5);
geometryToolTile.getGraphPoint().should("have.length", 2);
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.clickGraphPosition(10, 5);
geometryToolTile.getGraphPoint().should("have.length", 3);
geometryToolTile.addPointToGraph(10, 10);
geometryToolTile.clickGraphPosition(10, 10);
geometryToolTile.getGraphPoint().should("have.length", 4);
geometryToolTile.addPointToGraph(5, 5); // click first point again to close polygon.
geometryToolTile.clickGraphPosition(5, 5); // click first point again to close polygon.
geometryToolTile.getGraphPoint().should("have.length", 4);
geometryToolTile.getGraphPolygon().should("have.length", 1);

Expand All @@ -192,11 +192,30 @@ context('Geometry Tool', function () {
geometryToolTile.toggleAngleCheckbox();
geometryToolTile.getGraphPointLabel().contains('90°').should('exist');

// Label the polygon
geometryToolTile.getGraphPolygon().click(50, 50, { force: true, });
geometryToolTile.getSelectedGraphPoint().should('have.length', 3);
geometryToolTile.getGraphPointLabel().contains('12.5').should('not.exist');
geometryToolTile.getGraphPointLabel().contains('ABC').should('not.exist');
clueCanvas.clickToolbarButton('geometry', 'label');
geometryToolTile.getModalTitle().should('contain.text', 'Polygon Label/Value');
geometryToolTile.chooseLabelOption('length');
geometryToolTile.getGraphPointLabel().contains('12.5').should('exist');
clueCanvas.clickToolbarButton('geometry', 'label');
geometryToolTile.getModalLabelInput().should('have.value', 'ABC');
geometryToolTile.chooseLabelOption('label');
geometryToolTile.getGraphPointLabel().contains('12.5').should('not.exist');
geometryToolTile.getGraphPointLabel().contains('ABC').should('exist');
clueCanvas.clickToolbarButton('geometry', 'label');
geometryToolTile.chooseLabelOption('none');
geometryToolTile.clickGraphPosition(0, 0); // deselect polygon

// Label a segment
geometryToolTile.getGraphPointLabel().contains('AB').should('not.exist');
geometryToolTile.getGraphLine().should('have.length', 5); // 0-1 = axis lines, 2-4 = triangle
geometryToolTile.getGraphLine().eq(4).click({ force: true });
clueCanvas.clickToolbarButton('geometry', 'label');
geometryToolTile.getModalTitle().should('contain.text', 'Segment Label/Value');
geometryToolTile.chooseLabelOption('label');
geometryToolTile.getGraphPointLabel().contains('AB').should('exist');
clueCanvas.clickToolbarButton('geometry', 'label');
Expand Down
2 changes: 1 addition & 1 deletion cypress/e2e/smoke/single_student_canvas_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ context('single student functional test', () => {
cy.log('adds a geometry tool');
clueCanvas.addTile('geometry');
geometryToolTile.getGeometryTile().should('exist');
geometryToolTile.addPointToGraph(0, 0);
geometryToolTile.clickGraphPosition(0, 0);

cy.log('adds an image tool');
clueCanvas.addTile('image');
Expand Down
10 changes: 9 additions & 1 deletion cypress/support/elements/tile/GeometryToolTile.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class GeometryToolTile {
getGraphPolygon(){
return cy.get('.single-workspace .geometry-content.editable polygon');
}
addPointToGraph(x,y){
clickGraphPosition(x,y){
let transX=this.transformFromCoordinate('x', x),
transY=this.transformFromCoordinate('y', y);

Expand All @@ -123,6 +123,14 @@ class GeometryToolTile {
return cy.get('.geometry-menu-button');
}

getModalTitle() {
return cy.get('.ReactModalPortal');
}

getModalLabelInput() {
return cy.get('.ReactModalPortal input[type=text]');
}

// Name should be something like 'none', 'label', or 'length'
chooseLabelOption(name) {
cy.get(`.ReactModalPortal input[value=${name}]`).click();
Expand Down
49 changes: 36 additions & 13 deletions src/components/tiles/geometry/geometry-content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import { getClipboardContent, pasteClipboardImage } from "../../../utilities/cli
import { TileTitleArea } from "../tile-title-area";
import { GeometryTileContext } from "./geometry-tile-context";
import LabelPointDialog from "./label-point-dialog";
import LabelPolygonDialog from "./label-polygon-dialog";

export interface IGeometryContentProps extends IGeometryProps {
onSetBoard: (board: JXG.Board) => void;
Expand Down Expand Up @@ -91,6 +92,7 @@ interface IState extends Mutable<SizeMeProps> {
selectedLine?: JXG.Line;
showPointLabelDialog?: boolean;
showSegmentLabelDialog?: boolean;
showPolygonLabelDialog?: boolean;
showInvalidTableDataAlert?: boolean;
}

Expand Down Expand Up @@ -193,7 +195,6 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
handleDuplicate: this.handleDuplicate,
handleDelete: this.handleDelete,
handleLabelDialog: this.handleLabelDialog,
handleCreateLineLabel: this.handleCreateLineLabel,
handleCreateMovableLine: this.handleCreateMovableLine,
handleCreateComment: this.handleCreateComment,
handleUploadImageFile: this.handleUploadBackgroundImage,
Expand Down Expand Up @@ -538,6 +539,7 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
<>
{this.renderCommentEditor()}
{this.renderLineEditor()}
{this.renderPolygonLabelDialog()}
{this.renderSegmentLabelDialog()}
{this.renderPointLabelDialog()}
<div id={this.elementId} key="jsxgraph"
Expand Down Expand Up @@ -634,6 +636,28 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
}
}

private renderPolygonLabelDialog() {
const content = this.getContent();
const { board, showPolygonLabelDialog } = this.state;
if (board && showPolygonLabelDialog) {
const polygon = content.getOneSelectedPolygon(board);
if (!polygon) return;
const handleClose = () => this.setState({ showPolygonLabelDialog: false });
const handleAccept = (poly: JXG.Polygon, labelOption: ELabelOption, name: string) => {
this.handleLabelPolygon(poly, labelOption, name);
handleClose();
};
return (
<LabelPolygonDialog
board={board}
polygon={polygon}
onAccept={handleAccept}
onClose={handleClose}
/>
);
}
}

private renderRotateHandle() {
const { board, disableRotate } = this.state;
const selectedPolygon = board && !disableRotate && !this.props.readOnly
Expand Down Expand Up @@ -900,9 +924,13 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
return hasSelectedPoints;
};

private handleLabelDialog = (selectedPoint: JXG.Point|undefined, selectedSegment: JXG.Line|undefined) => {
private handleLabelDialog = (selectedPoint: JXG.Point|undefined, selectedSegment: JXG.Line|undefined,
selectedPolygon: JXG.Polygon|undefined) => {
// If there are just two points in a polygon, we want to label the segment not the polygon.
if (selectedSegment) {
this.setState({ showSegmentLabelDialog: true });
} else if (selectedPolygon) {
this.setState({ showPolygonLabelDialog: true });
} else {

Check warning on line 934 in src/components/tiles/geometry/geometry-content.tsx

View check run for this annotation

Codecov / codecov/patch

src/components/tiles/geometry/geometry-content.tsx#L934

Added line #L934 was not covered by tests
this.setState({ showPointLabelDialog: true });
}
Expand Down Expand Up @@ -967,17 +995,6 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
this.setState({ selectedLine: undefined });
};

private handleCreateLineLabel = () => {
const { board } = this.state;
const content = this.getContent();
if (board) {
const segment = content.getOneSelectedSegment(board);
if (segment) {
this.setState({ showSegmentLabelDialog: true });
}
}
};

// Currently, we don't allow commenting of polygon edges because the commenting feature
// requires that objects have persistent/unique IDs, but polygon edges don't have such
// IDs because their IDs are generated by JSXGraph.
Expand Down Expand Up @@ -1012,6 +1029,12 @@ export class GeometryContentComponent extends BaseComponent<IProps, IState> {
});
};

private handleLabelPolygon = (polygon: JXG.Polygon, labelOption: ELabelOption, name: string) => {
this.applyChange(() => {
this.getContent().updatePolygonLabel(this.state.board, polygon, labelOption, name);
});
};

private handleUpdateComment = (text: string, commentId?: string) => {
const { board } = this.state;
const content = this.getContent();
Expand Down
4 changes: 2 additions & 2 deletions src/components/tiles/geometry/geometry-shared.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import { HotKeyHandler } from "../../../utilities/hot-keys";
export interface IToolbarActionHandlers {
handleDuplicate: () => void;
handleDelete: () => void;
handleLabelDialog: (selectedPoint: JXG.Point|undefined, selectedSegment: JXG.Line|undefined ) => void;
handleLabelDialog: (selectedPoint: JXG.Point|undefined, selectedSegment: JXG.Line|undefined,
selectedPolygon: JXG.Polygon|undefined ) => void;
handleCreateMovableLine: () => void;
handleCreateLineLabel: () => void;
handleCreateComment: () => void;
handleUploadImageFile: (file: File) => void;
handleZoomIn: () => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,22 @@ const LabelButton = observer(function LabelButton({name}: IToolbarButtonComponen
const { content, board, handlers } = useGeometryTileContext();
const selectedPoint = board && content?.getOneSelectedPoint(board);
const selectedSegment = board && content?.getOneSelectedSegment(board);
const selectedPolygon = board && content?.getOneSelectedPolygon(board);

const pointHasLabel = selectedPoint && selectedPoint.hasLabel;
const segmentHasLabel = selectedSegment && selectedSegment.hasLabel;
const polygonHasLabel = selectedPolygon && selectedPolygon.hasLabel;

function handleClick() {
handlers?.handleLabelDialog(selectedPoint, selectedSegment);
handlers?.handleLabelDialog(selectedPoint, selectedSegment, selectedPolygon);
}

return (
<TileToolbarButton
name={name}
title="Label/Value"
disabled={!selectedPoint && !selectedSegment}
selected={pointHasLabel || segmentHasLabel}
disabled={!selectedPoint && !selectedSegment && !selectedPolygon}
selected={pointHasLabel || segmentHasLabel || polygonHasLabel}
onClick={handleClick}
>
<LabelSvg/>
Expand Down
31 changes: 31 additions & 0 deletions src/components/tiles/geometry/label-polygon-dialog.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React, { useEffect } from "react";
import { ELabelOption } from "../../../models/tiles/geometry/jxg-changes";
import { useLabelPolygonDialog } from "./use-label-polygon-dialog";

interface IProps {
board: JXG.Board;
polygon: JXG.Polygon;
onAccept: (polygon: JXG.Polygon, labelOption: ELabelOption, name: string) => void;
onClose: () => void;
}

// Component wrapper for useLabelPolygonDialog() for use by class components.
const LabelPolygonDialog: React.FC<IProps> = ({
board, polygon, onAccept, onClose
}: IProps) => {

const [showDialog, hideDialog] = useLabelPolygonDialog({
board,
polygon,
onAccept,
onClose
});

useEffect(() => {
showDialog();
return () => hideDialog();
}, [hideDialog, showDialog]);

return null;
};
export default LabelPolygonDialog;
Loading

0 comments on commit 270ef60

Please sign in to comment.