Skip to content

docs(changeset): Interactive Graph code cleaup and bug fix for edge-based label positioning. #2635

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Jul 2, 2025
Merged

docs(changeset): Interactive Graph code cleaup and bug fix for edge-based label positioning. #2635

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ddb18e6
[label-without-a-cause] Massive cleanup + bug fix
SonicScrewdriver Jun 23, 2025
e1563f1
[label-without-a-cause] docs(changeset): Interactive Graph code cleau…
SonicScrewdriver Jun 23, 2025
eee1b63
[label-without-a-cause] Minor improvements to base positioning.
SonicScrewdriver Jun 24, 2025
06e0465
Update packages/perseus/src/widgets/interactive-graphs/backgrounds/ut…
SonicScrewdriver Jun 26, 2025
76d3708
[label-without-a-cause] Fix another edge case
SonicScrewdriver Jun 27, 2025
5a2d1ac
[label-without-a-cause] Might have added extra padding in here
SonicScrewdriver Jun 27, 2025
1381836
[label-without-a-cause] Maybe it was 1.25
SonicScrewdriver Jun 27, 2025
f3c4107
[label-without-a-cause] Fix 0.04
SonicScrewdriver Jun 27, 2025
c0a7d2e
[label-without-a-cause] pr requests
SonicScrewdriver Jul 2, 2025
77adf83
[label-without-a-cause] increase threshold
SonicScrewdriver Jul 2, 2025
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
24 changes: 12 additions & 12 deletions packages/perseus/src/widgets/interactive-graphs/backgrounds/axis-labels.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ describe("getLabelPosition", () => {
};
const labelLocation = "onAxis";
const expected = [
[400, 400 + 1.25 * fontSize], // X Label at [Right edge of the graph, vertical center of the graph]
[-1.5 * fontSize, -2 * fontSize], // Y Label at [Horizontal center of the graph, 2x fontSize above the top edge]
[400, 400 + 1.25 * fontSize],
[-1.5 * fontSize, -2 * fontSize],
];

expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
Expand All @@ -79,8 +79,8 @@ describe("getLabelPosition", () => {
};
const labelLocation = "onAxis";
const expected = [
[400, -2 * fontSize], // X Label at [Right edge of the graph, vertical center of the graph]
[400 + 1.25 * fontSize, -2 * fontSize], // Y Label at [Horizontal center of the graph, 2x fontSize above the top edge]
[400, -2 * fontSize],
[400 + 1.25 * fontSize, -2 * fontSize],
];

expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
Expand All @@ -99,8 +99,8 @@ describe("getLabelPosition", () => {
};
const labelLocation = "alongEdge";
const expected = [
[200, 400 + fontSize * 1.5], // X Label at [Horizontal center of the graph, 1x fontSize below the bottom edge]
[-fontSize * 1.5, 200 - fontSize], // Y label at [1x fontSize to the left of the left edge, vertical center of the graph]
[200, 400 + fontSize * 1.5],
[-fontSize * 1.25, 200 - fontSize],
];

expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
Expand All @@ -119,8 +119,8 @@ describe("getLabelPosition", () => {
};
const labelLocation = "alongEdge";
const expected = [
[200, 400 + fontSize * 1.5], // X Label at [Horizontal center of the graph, 1x fontSize below the bottom edge]
[-fontSize * 1.5, 200 - fontSize], // Y label at [1x fontSize to the left of the left edge, vertical center of the graph]
[200, 400 + fontSize * 1.5],
[-fontSize * 1.25, 200 - fontSize],
];

expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
Expand All @@ -139,8 +139,8 @@ describe("getLabelPosition", () => {
};
const labelLocation = "alongEdge";
const expected = [
[200, 400 + 3 * fontSize], // X Label at [Horizontal center of the graph, 3x fontSize below the bottom edge]
[-3 * fontSize, 200 - fontSize], // Y label at [3x fontSize to the left of the left edge, vertical center of the graph]
[200, 400 + 3 * fontSize],
[-2.75 * fontSize, 200 - fontSize],
];

expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
Expand All @@ -160,8 +160,8 @@ describe("getLabelPosition", () => {
};
const labelLocation = "alongEdge";
const expected = [
[200, 400 + 3 * fontSize], // X Label at [Horizontal center of the graph, 3x fontSize below the bottom edge]
[-3 * fontSize, 200 - fontSize], // Y label at [3x fontSize to the left of the left edge, vertical center of the graph]
[200, 400 + 3 * fontSize],
[-2.75 * fontSize, 200 - fontSize],
];

expect(getLabelPosition(graphInfo, labelLocation, [1, 1])).toEqual(
Expand Down
18 changes: 14 additions & 4 deletions packages/perseus/src/widgets/interactive-graphs/backgrounds/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,16 @@ export const getLabelPosition = (
? [0, fontSize * 3] // Move the label down by 2 font sizes if the y-axis min is positive
: [0, fontSize * 1.5]; // Move the label down by 1.5 font sizes if the y-axis min is negative

// Determine if the x-axis min is negative and close to zero.
const isCloseToZero =
graphInfo.range[X][MIN] < 0 && graphInfo.range[X][MIN] > -0.05;
// Determine if the x-axis min is negative and relatively close to zero, based on the scale of the x-axis range.
const isRelativelyCloseToZero =
graphInfo.range[X][MIN] < 0 &&
Math.abs(graphInfo.range[X][MIN]) <
(graphInfo.range[X][MAX] - graphInfo.range[X][MIN]) * 0.05;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yessss


// Determine if the tick labels extend beyond the left edge of the graph, either
// because the x-axis is wholly positive or because the x-axis min is negative and close to zero.
const needsExtraSpacing = graphInfo.range[X][MIN] >= 0 || isCloseToZero;
const needsExtraSpacing =
graphInfo.range[X][MIN] >= 0 || isRelativelyCloseToZero;

// When tick labels extend beyond the left edge of the graph, we need to account for their
// width to prevent the main axis label from overlapping with them.
Expand Down Expand Up @@ -140,6 +143,13 @@ export const getLabelPosition = (
(graphInfo.range[Y][MIN] + graphInfo.range[Y][MAX]) / 2,
];

// If we're VERY close to zero, we want to account for the strange grid offset to avoid
// the y-axis label from getting too far from the tick labels.
if (isRelativelyCloseToZero) {
yAxisLabelLocation[X] =
yAxisLabelLocation[X] - graphInfo.range[X][MIN];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YESSSSS


// Convert the Vector2 coordinates to pixel coordinates and add the offsets
const xLabel = vec.add(
pointToPixel(xAxisLabelLocation, graphInfo),
Expand Down
15 changes: 15 additions & 0 deletions packages/perseus/src/widgets/interactive-graphs/interactive-graph-regression.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,21 @@ export const MafsWithLabelsAlongEdgeWithCloseToZeroXMin: Story = {
},
};

export const MafsWithLabelsAlongEdgeWithCloseToZeroXMinMultipliedBy1000: Story =
{
args: {
question: interactiveGraphQuestionBuilder()
.withXRange(-30, 840)
.withYRange(-2.8, 63)
.withTickStep(200, 10)
.withGridStep(50, 5)
.withSnapStep(25, 2)
.withAxisLabels("Time (seconds)", "Distance (meters)")
.withLabelLocation("alongEdge")
.build(),
},
};

export const MafsWithLabelsAlongEdgeZoomed: Story = {
args: {
question: interactiveGraphQuestionBuilder()
Expand Down
Loading