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

Geometry: Select, Points, and Polygon modes #2303

Merged
merged 25 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
aaf0a2a
Add no-op select button
bgoldowsky May 9, 2024
d5b2dec
Merge branch '186143166-geometry-toolbar' into 187496223-geometry-select
bgoldowsky May 10, 2024
298f754
Select mode doesn't create points on click
bgoldowsky May 10, 2024
9980520
First shot at phantom point tracking
bgoldowsky May 10, 2024
681ad67
Merge branch '186143166-geometry-toolbar' into 187496223-geometry-select
bgoldowsky May 13, 2024
fd3e47f
Improve colors, labels
bgoldowsky May 13, 2024
1825a94
First draft of polygon drawing mode
bgoldowsky May 14, 2024
0826bef
Working polygon mode
bgoldowsky May 15, 2024
46946d8
Fix missing-edge bug by updating JSXGraph a bit
bgoldowsky May 15, 2024
4108fe9
Handle mouse left/entered; try not rebuilding polygon
bgoldowsky May 17, 2024
71c6b95
Revert to brute-force polygon updates, older JSXGraph
bgoldowsky May 17, 2024
46afb73
Some cleanups. Mostly fix Jest tests
bgoldowsky May 20, 2024
c46e40c
Passing tests. A few TODOs remain
bgoldowsky May 20, 2024
0e03de7
Update a few tests
bgoldowsky May 20, 2024
1ec2e7c
Cleanups
bgoldowsky May 20, 2024
560784d
Logging updates
bgoldowsky May 22, 2024
9a18ce5
Merge branch '186143166-geometry-toolbar' into 187496223-geometry-select
bgoldowsky May 23, 2024
7725fac
Update CSS. Fix Cypress test
bgoldowsky May 23, 2024
9421f22
Don't hide first edge drawn
bgoldowsky May 23, 2024
ebf47f8
PR comments; doc update
bgoldowsky May 28, 2024
6ff8855
Change default to select mode
bgoldowsky May 28, 2024
acb6887
Remove 'highlight' effects (aka 'flicker')
bgoldowsky May 28, 2024
8beb526
Fix test
bgoldowsky May 28, 2024
67a3606
Cypress tests update
bgoldowsky May 28, 2024
9198b7d
Revert selected color
bgoldowsky May 28, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ context('Copy Document', () => {
geometryTile.addPointToGraph(5, 5);
geometryTile.addPointToGraph(10, 5);
geometryTile.addPointToGraph(10, 10);
geometryTile.getGraphPoint().should('have.length', 3);
geometryTile.getGraphPoint().should('have.length', 4);

cy.log('Add drawing tile');
clueCanvas.addTile("drawing");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function setupTest(studentIndex) {
geometryToolTile.addPointToGraph(5, 5);
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.addPointToGraph(10, 10);
geometryToolTile.getGraphPoint().should('have.length', 3);
geometryToolTile.getGraphPoint().should('have.length', 4); // including phantom point
clueCanvas.addTile("drawing");
drawToolTile.getDrawToolRectangle().click();
drawToolTile.getDrawTile()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ context('Test copy tiles from one document to other document', function () {
geometryToolTile.addPointToGraph(5, 5);
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.addPointToGraph(10, 10);
geometryToolTile.getGraphPoint().should('have.length', 3);
geometryToolTile.getGraphPoint().should('have.length', 4);

cy.log('Add drawing tile');
clueCanvas.addTile("drawing");
Expand Down
18 changes: 11 additions & 7 deletions cypress/e2e/functional/tile_tests/arrow_annotation_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -236,21 +236,23 @@ context('Arrow Annotations (Sparrows)', function () {
aa.getAnnotationArrows().should("have.length", 4);
});

it("can add arrows to geometry tiles", () => {
it.only("can add arrows to geometry tiles", { scrollBehavior: 'nearest'}, () => {
bgoldowsky marked this conversation as resolved.
Show resolved Hide resolved
beforeTest(queryParams);
clueCanvas.addTile("geometry");

cy.log("Annotation buttons appear for points, polygons, and segments");
clueCanvas.clickToolbarButton('geometry', 'polygon');
aa.clickArrowToolbarButton(); // sparrow mode on
aa.getAnnotationLayer().should("have.class", "editing");
aa.getAnnotationButtons().should("not.exist");

aa.clickArrowToolbarButton(); // sparrow mode off
// For some reason adding the first point is ignored, so we add four but get three to make a triangle
geometryToolTile.addPointToGraph(5, 5);
geometryToolTile.getGeometryTile().click(); // select tile
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.addPointToGraph(15, 10);
geometryToolTile.addPointToGraph(20, 5);
geometryToolTile.getGraphPoint().last().dblclick({ force: true });
geometryToolTile.addPointToGraph(10, 5); // close polygon

aa.clickArrowToolbarButton(); // sparrow mode on
// 3 points + 3 segments + 1 polygon = 7
aa.getAnnotationButtons().should("have.length", 7);
Expand All @@ -261,14 +263,16 @@ context('Arrow Annotations (Sparrows)', function () {
aa.getAnnotationButtons().eq(6).click();
aa.getAnnotationArrows().should("have.length", 1);
aa.getAnnotationDeleteButtons().eq(0).click();

// Remove all the points and polygons
aa.clickArrowToolbarButton(); // sparrow mode off
geometryToolTile.getGeometryTile().click(); // select tile
geometryToolTile.getGraphPoint().eq(2).click();
geometryToolTile.deleteGraphElement();
clueCanvas.clickToolbarButton('geometry', 'delete');
geometryToolTile.getGraphPoint().eq(1).click();
geometryToolTile.deleteGraphElement();
clueCanvas.clickToolbarButton('geometry', 'delete');
geometryToolTile.getGraphPoint().eq(0).click();
geometryToolTile.deleteGraphElement();
clueCanvas.clickToolbarButton('geometry', 'delete');
aa.getAnnotationButtons().should("have.length", 0);
aa.getAnnotationArrows().should("have.length", 0);

Expand Down
56 changes: 56 additions & 0 deletions cypress/e2e/functional/tile_tests/geometry_tool_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,62 @@ context('Geometry Tool', function () {
geometryToolTile.getGraphPoint().should('have.length', 3);
});

it.only('works in all three modes', () => {
bgoldowsky marked this conversation as resolved.
Show resolved Hide resolved
beforeTest();
clueCanvas.addTile('geometry');
geometryToolTile.getGraph().should("exist");

cy.log("add points with points mode");
clueCanvas.clickToolbarButton('geometry', 'point');
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.getGraphPoint().should("have.length", 3);

cy.log("select points with select mode");
clueCanvas.clickToolbarButton('geometry', 'select');
clueCanvas.toolbarButtonIsSelected('geometry', 'select');
geometryToolTile.getGraph().trigger('mousemove');
geometryToolTile.getGraphPoint().should("have.length", 2); // no phantom point

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

geometryToolTile.getSelectedGraphPoint().should("have.length", 0);
// FIXME Not working. Return to this when we update the design for selected points.
// cy.log("select first");
// geometryToolTile.selectGraphPoint(1, 1, true);
// geometryToolTile.getGraphPoint().eq(0).should("have.attr", "stroke", "#FF0000");
// geometryToolTile.getSelectedGraphPoint().should("have.length", 1);
// cy.log("select second");
// geometryToolTile.selectGraphPoint(2, 2);
// geometryToolTile.getSelectedGraphPoint().should("have.length", 1);
// cy.log("select both");
// geometryToolTile.selectGraphPoint(1, 1, true);
// geometryToolTile.getSelectedGraphPoint().should("have.length", 2);

geometryToolTile.selectGraphPoint(1, 1);
clueCanvas.clickToolbarButton('geometry', 'delete');
geometryToolTile.getGraphPoint().should("have.length", 1);
geometryToolTile.selectGraphPoint(2, 2);
clueCanvas.clickToolbarButton('geometry', 'delete');
geometryToolTile.getGraphPoint().should("have.length", 0);

cy.log("make a polygon with polygon mode");
clueCanvas.clickToolbarButton('geometry', 'polygon');
clueCanvas.toolbarButtonIsSelected('geometry', 'polygon');
geometryToolTile.getGraph().trigger('mousemove');
geometryToolTile.getGraphPoint().should("have.length", 1); // phantom point
geometryToolTile.addPointToGraph(5, 5);
geometryToolTile.addPointToGraph(10, 5);
geometryToolTile.addPointToGraph(9, 9);
geometryToolTile.addPointToGraph(5, 5); // click first point again to close polygon.
geometryToolTile.getGraphPolygon().should("have.length", 1);
});

it('will test Geometry tile undo redo', () => {
beforeTest();

Expand Down
12 changes: 7 additions & 5 deletions cypress/support/elements/tile/GeometryToolTile.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,23 @@ class GeometryToolTile {
getGraphPoint(){
return cy.get('.geometry-content.editable ellipse[display="inline"]');
}
getSelectedGraphPoint() {
// TODO: when we update the design, should make this a CSS class
return cy.get('.geometry-content.editable ellipse[stroke="#FF0000"]');
}
hoverGraphPoint(x,y){
let transX=this.transformFromCoordinate('x', x),
transY=this.transformFromCoordinate('y', y);

this.getGraph().last()
.trigger('mouseover',transX,transY);
}
selectGraphPoint(x,y){
selectGraphPoint(x, y, withShiftKey = false ){
let transX=this.transformFromCoordinate('x', x),
transY=this.transformFromCoordinate('y', y);

this.getGraph().last().click(transX, transY, {force:true});
this.getGraph().last()
.click(transX, transY, { force:true, shiftKey: withShiftKey });
}
getGraphPointID(point){
return cy.get('.geometry-content.editable ellipse').eq(point)
Expand Down Expand Up @@ -128,8 +133,5 @@ class GeometryToolTile {
addComment(){
cy.get('.single-workspace.primary-workspace .geometry-toolbar .button.comment.enabled').click();
}
deleteGraphElement(){
cy.get('.single-workspace.primary-workspace .geometry-toolbar .button.delete.enabled').click();
}
}
export default GeometryToolTile;
2 changes: 1 addition & 1 deletion dependencies-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Notes on dependencies, particularly reasons for not updating to their latest ver
|chart.js |2.9.4 |3.9.1 |Major version not attempted; only used by Dataflow tile, which doesn't really use it.|
|firebase |8.10.1 |9.9.3 |Version 9 requires substantial migration; attempted update with `compat` imports failed.|
|immutable |3.8.2 |4.1.0 |Major version update not attempted; only required by legacy slate versions. |
|jsxgraph |1.4.4 |1.4.5 |1.4.5 broke scaled rendering, e.g. in 4-up views |
|jsxgraph |1.4.4 |1.8.0 |1.4.5 broke scaled rendering, e.g. in 4-up views |
|mob-state-tree |5.1.5-cc.1 |5.1.6 |We are using a concord fork which fixes a bug. Additionally latest version changes TS types for arrays which broke a number of our models.|
|nanoid |3.3.4 |4.0.0 |v4 switched to ESM and dependencies such as postcss break with v4 |
|netlify-cms-app |2.15.72 |2.15.72 |Requires React 16 or 17. Blocks upgrade to React 18.
Expand Down
7 changes: 5 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions src/clue/app-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,9 @@
},
"geometry": {
"tools": [
"select",
"point",
"polygon",
"upload",
"duplicate",
"angle-label",
Expand Down
15 changes: 15 additions & 0 deletions src/clue/assets/icons/geometry/polygon-icon.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Loading