Numberline claud by lbondaryk · Pull Request #2722 · concord-consortium/collaborative-learning

lbondaryk · 2026-01-21T14:31:38Z

This adds point value labels to the numberline when points are selected. Labels update when points are dragged and disappear when points are deselected. Some accessibility has been applied in terms of implementing tabbing through fields in the numberline tile.

- Zero tick mark always appears when min < 0 and max > 0 - Zero tick has bold styling even when not part of regular tick spacing - Zero label only shows when zero falls on regular tick spacing - Added Cypress tests for zero tick behavior

…bility with ARIA labels.

codecov · 2026-01-21T16:38:42Z

Codecov Report

❌ Patch coverage is 57.37705% with 52 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.23%. Comparing base (d5f8899) to head (ef7b453).
⚠️ Report is 17 commits behind head on master.

Files with missing lines	Patch %	Lines
.../plugins/numberline/components/numberline-tile.tsx	62.19%	31 Missing ⚠️
...umberline/components/editable-numberline-value.tsx	44.00%	14 Missing ⚠️
...rc/plugins/numberline/models/numberline-content.ts	22.22%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2722      +/-   ##
==========================================
- Coverage   86.78%   86.23%   -0.56%     
==========================================
  Files         811      811              
  Lines       43429    43529     +100     
  Branches    11115    11144      +29     
==========================================
- Hits        37689    37536     -153     
- Misses       5408     5655     +247     
- Partials      332      338       +6

Flag	Coverage Δ
cypress	`?`
cypress-regression	`77.72% <57.37%> (+0.50%)`	⬆️
cypress-smoke	`?`
jest	`49.44% <40.00%> (+0.07%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

cypress · 2026-01-21T16:39:17Z

collaborative-learning Run #17391

Run Properties: Failed #17391 • ef7b453ca4: Fix Cypress test selector to avoid multiple element match.

Project	`collaborative-learning`
Branch Review	`numberlineClaud`
Run status	`Failed #17391`
Run duration	`48m 38s`
Commit	`ef7b453ca4: Fix Cypress test selector to avoid multiple element match.`
Committer	`lbondaryk`
View all properties for this run ↗︎

Test results
Failures	`1`
Flaky	`4`
Pending	`4`
Skipped	`0`
Passing	`149`
View all changes introduced in this branch ↗︎

Tests for review

cypress/e2e/functional/tile_tests/numberline_tool_spec.js • 1 failed test • Regression tests

View Output

Test		Artifacts
Numberline Tile > Numberline Tile		`Test Replay` `Screenshots`

xy_plot_tool_spec.js • 2 flaky tests • Regression tests

View Output

Test		Artifacts
XYPlot Tool Tile > XYPlot Tool > XYPlot tool tile		`Test Replay` `Screenshots`
XYPlot Tool Tile > XYPlot Tool > Test duplicating graph with an xy-plot (a.k.a. graph)		`Test Replay` `Screenshots`

arrow_annotation_spec.js • 1 flaky test • Regression tests

View Output

Test		Artifacts
Arrow Annotations (Sparrows) > can add arrows to xy plot tiles		`Test Replay` `Screenshots`

geometry_tool_spec.js • 1 flaky test • Regression tests

View Output

Test		Artifacts
Geometry Tool > works in all four modes		`Test Replay` `Screenshots`

tealefristoe

The biggest issue here is that the cypress test is failing. The first new test is failing, so it's very possible multiple tests are failing afterwards.

But there are a lot of places where this should be adjusted. Part of the issue is that the nummberline tile is not very well structured to begin with, so building on top of it is going to exacerbate the mess. But also the AI seems to not understand how to keep things efficient, or separate new functionality between the model and component.

Looking over this, I feel like there are some easy, generic additions we could use for all CLUE prompts to improve the resulting code, such as "use variables defined in vars.scss rather than hard-coded color values whenever possible" and "make sure lines are limited to 121 characters at most".

tealefristoe · 2026-01-21T16:25:30Z

cypress/e2e/functional/tile_tests/numberline_tool_spec.js

+    cy.get(".numberline-wrapper").click().type('{tab}');
+    numberlineToolTile.getValueLabel().should("exist");


Suggested change

cy.get(".numberline-wrapper").click().type('{tab}');

numberlineToolTile.getValueLabel().should("exist");

cy.get(".numberline-wrapper").click().type('{tab}');

numberlineToolTile.getValueLabel().should("exist");

numberlineToolTile.getValueLabelText().should("contain", "-2.00");

tealefristoe · 2026-01-21T16:39:28Z

cypress/e2e/functional/tile_tests/numberline_tool_spec.js

+    // Zero tick should exist even if not in regular spacing
+    cy.get(".numberline-tool-container .zero-tick").should("exist");
+    // Zero should not have a label since it's not part of regular tick spacing
+    numberlineToolTile.getAllNumberlineTicks().should('not.contain', '0');


The cypress test is failing here, so nothing below is actually being run at this point.

tealefristoe · 2026-01-21T16:40:24Z

cypress/e2e/functional/tile_tests/numberline_tool_spec.js

+    cy.log("will edit min value with keyboard");
+    // Click to show input
+    numberlineToolTile.getMinBox().click();
+    numberlineToolTile.getMinBoxInput().should('exist');
+    numberlineToolTile.getMinBoxInput().clear().type('-8{enter}');
+    numberlineToolTile.getMinBox().should('contain', '-8');
+
+    cy.log("will edit max value with keyboard");
+    // Click to show input
+    numberlineToolTile.getMaxBox().click();
+    numberlineToolTile.getMaxBoxInput().should('exist');
+    numberlineToolTile.getMaxBoxInput().clear().type('8{enter}');
+    numberlineToolTile.getMaxBox().should('contain', '8');


Are these tests redundant, since the range is already being changed above (like on lines 49 and 50)?

tealefristoe · 2026-01-21T16:45:19Z

src/plugins/numberline/components/editable-numberline-value.tsx

+      onFocus={handleFocus}
+      tabIndex={readOnly ? -1 : 0}
+      role="button"
+      aria-label={`${minOrMax === "min" ? "Minimum" : "Maximum"} value: ${value}. ${readOnly ? "" : "Press Enter to edit."}`}


Define const minOrMaxWord = minOrMax === "min" ? "Minimum" : "Maximum"; above, then use it here to make this line below the max length. You can also use minOrMaxWord on line 110 below.

tealefristoe · 2026-01-21T16:47:35Z

src/plugins/numberline/components/numberline-tile.scss

+
+        .point-value-label-bg {
+          fill: $workspace-teal-light-8;
+          stroke: #949494;


Suggested change

stroke: #949494;

stroke: $charcoal-light-1;

tealefristoe · 2026-01-21T17:58:42Z

src/plugins/numberline/components/numberline-tile.tsx

+          .attr("rx", 10)
+          .attr("ry", 10);


A constant should be used instead of hard coded 10 here.

tealefristoe · 2026-01-21T18:01:02Z

src/plugins/numberline/components/numberline-tile.tsx

+          .attr("stroke", "#949494")
+          .attr("stroke-width", 1.5)
+          .style("pointer-events", "none");


Could these be defined in css instead?

tealefristoe · 2026-01-21T18:04:34Z

src/plugins/numberline/models/numberline-content.test.ts

+    it("only allows one selected point at a time", () => {
+      const content = NumberlineContentModel.create();
+      const point1 = content.createAndSelectPoint(1.0, false);
+      const point2 = content.createAndSelectPoint(3.0, false);
+
+      expect(content.points.size).toBe(2);
+      expect(content.selectedPoints[point1.id]).toBeUndefined();
+      expect(content.selectedPoints[point2.id]).toBe(point2);
+      expect(Object.keys(content.selectedPoints).length).toBe(1);
+    });


If we only want a single point to be selectable at a time, we should just take out the infrastructure that supports multiple points being selected.

tealefristoe · 2026-01-21T18:05:05Z

src/plugins/numberline/models/numberline-content.test.ts

+
+    it("clears all selected points", () => {
+      const content = NumberlineContentModel.create();
+      const point = content.createAndSelectPoint(2.0, false);


Suggested change

const point = content.createAndSelectPoint(2.0, false);

const _point = content.createAndSelectPoint(2.0, false);

To avoid a lint warning.

tealefristoe · 2026-01-21T18:08:09Z

src/plugins/numberline/models/numberline-content.test.ts

+    it("can select points in order by position", () => {
+      const content = NumberlineContentModel.create();
+      const pointA = content.createNewPoint(3.0, false);
+      const pointB = content.createNewPoint(-2.0, false);
+      const pointC = content.createNewPoint(1.0, false);
+
+      // Points sorted by xValue: B(-2), C(1), A(3)
+      const sortedPoints = content.pointsArr.slice().sort((a, b) => a.xValue - b.xValue);
+      expect(sortedPoints[0].id).toBe(pointB.id);
+      expect(sortedPoints[1].id).toBe(pointC.id);
+      expect(sortedPoints[2].id).toBe(pointA.id);
+    });


This test isn't particularly useful. Once sortedPointsArr (or something similar) is added to NumberlineContentModel, it will make sense to test that.

- Fix Cypress tests: use exact equality checks, add missing assertions, remove redundant tests - Replace hard-coded colors with vars.scss variables for consistency - Fix lines exceeding 121 character limit by extracting repeated strings into variables - Move logic to model layer: add sortedPointsArr, selectedPointsArr, and firstSelectedPoint views - Improve efficiency: combine tick generation into single O(n) pass, eliminate redundant sorting - Remove duplicate keyboard event handlers (HotKeys vs D3 handlers) - Support moving all selected points with arrow keys (future-proofing for multi-select) - Add kValueLabelBorderRadius constant, move inline styles to CSS Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Values very close to zero (between -0.005 and 0.005) now display as "0.00" instead of potentially showing "-0.00". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Scoped .numberline-wrapper selector to .primary-workspace to ensure only one element is matched when clicking/typing on the tile. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

lbondaryk added 5 commits January 16, 2026 15:00

Adds point labels to numberlines when points are selected.

a9c309a

tests for numberline labels.

e5aaef1

Adds keyboard navigation to Numberline Tool tile and improves accessi…

6170827

…bility with ARIA labels.

Tests and other accessibility improvements for numberline tool.

1a19423

lbondaryk requested a review from tealefristoe January 21, 2026 14:31

lbondaryk assigned emcelroy Jan 21, 2026

tealefristoe marked this pull request as draft January 21, 2026 16:26

tealefristoe added the run regression label Jan 21, 2026

tealefristoe requested changes Jan 21, 2026

View reviewed changes

github-actions bot temporarily deployed to development January 23, 2026 19:51 Inactive

Fix negative zero display in numberline point labels.

0b9aa01

Values very close to zero (between -0.005 and 0.005) now display as "0.00" instead of potentially showing "-0.00". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

github-actions bot temporarily deployed to development January 23, 2026 20:06 Inactive

Fix Cypress test selector to avoid multiple element match.

ef7b453

Scoped .numberline-wrapper selector to .primary-workspace to ensure only one element is matched when clicking/typing on the tile. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

github-actions bot temporarily deployed to development January 23, 2026 20:48 Inactive

		cy.get(".numberline-wrapper").click().type('{tab}');
		numberlineToolTile.getValueLabel().should("exist");

	const point = content.createAndSelectPoint(2.0, false);
	const _point = content.createAndSelectPoint(2.0, false);

Conversation

lbondaryk commented Jan 21, 2026

Uh oh!

codecov bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cypress bot commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests for review

Uh oh!

tealefristoe left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Jan 21, 2026 •

edited

Loading

cypress bot commented Jan 21, 2026 •

edited

Loading

tealefristoe left a comment •

edited

Loading