[AbsoluteValue] PR3 - rendering & accessibility#3350
[AbsoluteValue] PR3 - rendering & accessibility#3350handeyeco wants to merge 6 commits intoLEMS-3347/absolute-value-pr2from
Conversation
🗄️ Schema Change: No Changes ✅ |
handeyeco
changed the title
github-actions
bot
added
item-splitting-change
schema-change
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (16655f3) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3350If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3350If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3350 |
github-actions
bot
removed
the
schema-change
|
Size Change: +1.31 kB (+0.27%) Total Size: 488 kB
ℹ️ View Unchanged
|
🛠️ Item Splitting: No Changes ✅ |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| const coeffs = getAbsoluteValueCoefficients(coords); | ||
| const m = coeffs?.m ?? 1; | ||
| const vertexStr = `${srFormatNumber(vertex[X], locale)} comma ${srFormatNumber(vertex[Y], locale)}`; |
There was a problem hiding this comment.
🟡 The vertex coordinate string in describeAbsoluteValueGraph (line 202) hardcodes the English word "comma" in a template literal, which is then passed as the vertex param to srAbsoluteValueDescription. Because "comma" is embedded inside the interpolated value, it is invisible to translators and will appear untranslated for non-English locales. The fix is to split vertex into separate vertexX/vertexY params in the i18n string, matching the pattern already used by srAbsoluteValueVertexPoint and the angle graph's srAngleGraphAriaDescription.
Extended reasoning...
What the bug is
In describeAbsoluteValueGraph at absolute-value.tsx:202, the vertex coordinate string is built with a hardcoded English word:
const vertexStr = `${srFormatNumber(vertex[X], locale)} comma ${srFormatNumber(vertex[Y], locale)}`;This vertexStr is then passed as the vertex parameter to strings.srAbsoluteValueDescription({vertex: vertexStr, slope: ...}). The translatable message template is:
"The graph shows an absolute value function with vertex at %(vertex)s and slope %(slope)s."
Since "comma" is embedded inside the %(vertex)s placeholder value, it is part of the JavaScript code, not part of the translatable message. Translators never see it and cannot translate it.
Why existing code does not prevent this
The i18n translation system (used via strings.*) only exposes the message template and its named placeholders to translators. Translators can translate words in the template itself (like "vertex at" or "slope"), but they have no visibility into or control over what values are substituted into placeholders at runtime. The word "comma" is baked into the runtime value, bypassing the translation pipeline entirely.
Proof by comparison with correct patterns
Consider how the angle graph handles the same situation in angle.tsx:202-207:
const srAngleGraphAriaDescription = strings.srAngleGraphAriaDescription({
angleMeasure,
vertexX: srFormatNumber(vertex[X], locale),
vertexY: srFormatNumber(vertex[Y], locale),
...
});The corresponding message template is:
"The angle measure is %(angleMeasure)s degrees with a vertex at %(vertexX)s comma %(vertexY)s..."
Here, "comma" is inside the translatable message string itself, so translators can replace it with the appropriate word in their language. Even within this same file, srAbsoluteValueVertexPoint uses the correct pattern: "Vertex point at %(x)s comma %(y)s." with "comma" in the template, not in the interpolated value.
Impact
For any non-English locale, screen reader users will hear the English word "comma" in the middle of a sentence that is otherwise translated into their language. For example, a Spanish user might hear: "El grafico muestra una funcion de valor absoluto con vertice en 3 comma 5 y pendiente 2" where "comma" should be the locale-appropriate word.
Recommended fix
Refactor srAbsoluteValueDescription to accept vertexX and vertexY as separate parameters, and move "comma" into the translatable message template:
message: "The graph shows an absolute value function with vertex at %(vertexX)s comma %(vertexY)s and slope %(slope)s."
This matches the existing pattern used by srAbsoluteValueVertexPoint, srAbsoluteValueSecondPoint, srAbsoluteValueInteractiveElements, and the angle graph's description string.
github-actions
bot
added
item-splitting-change
and removed
item-splitting-change
labels
1 participant
Everything is AI
Summary:
This is part of a series of PRs as defined in the first PR:
Initial implementation experiment is Perseus#3304.
Issue: LEMS-3347
Implement rendering and accessibility for absolute-value graph type
Adds the visual component, screen reader strings, equation display, and
unit tests for the absolute-value graph. Depends on PR 1 + PR 2. After this
PR the graph is fully interactive and renderable.
Changes:
renderAbsoluteValueGraph, the AbsoluteValueGraph component, and two
exported helpers:
control points, returning undefined if both share the same x. Uses a
coeffRef to cache the last valid result for transient mid-drag states.
keyboard movement constraint that skips any position where the moved point
would share an x-coordinate with the other point.
two components with appropriate aria labels and an
description.
coefficient extraction (upward/downward V, varying slopes, same-x returns
undefined, left/right arm equivalence), keyboard constraint skip behaviour,
and all screen reader strings (graph label, point labels, description,
interactive elements text).
srAbsoluteValueVertexPoint, srAbsoluteValueSecondPoint,
srAbsoluteValueDescription, srAbsoluteValueInteractiveElements) with i18n
context comments, English defaults, and mockStrings implementations.
renderAbsoluteValueGraph.
(returns y = m.toFixed(3)|x - h.toFixed(3)| + v.toFixed(3)) and wires it
into getEquationString.
required by the new component file.