Skip to content

[CLASS-13991] Stop swapping scroll caret icons on isRTL - #4104

Draft
maddy531 wants to merge 3 commits into
mainfrom
CLASS-13991.rtl-icon-swaps
Draft

[CLASS-13991] Stop swapping scroll caret icons on isRTL#4104
maddy531 wants to merge 3 commits into
mainfrom
CLASS-13991.rtl-icon-swaps

Conversation

@maddy531

@maddy531 maddy531 commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

PhosphorIcon mirrors directional glyphs automatically in RTL once the
wonder-blocks-icon major lands (Khan/wonder-blocks#3174). This adapts perseus
to that, in the two places it matters:

scrollable-view.tsx — picking the caret from isRTL double-flips against
auto-mirroring, so it now passes the LTR-facing carets and lets mirroring handle
direction. That leaves isRTL with no readers on the file-local ScrollState
interface, so it's dropped; newIsRtl still drives the
canScrollStart/canScrollEnd math and stays.

locked-polygon-settings.tsx — the "move polygon left/right" arrows point
along the graph's x-axis, which is absolute: "left" means decreasing x in every
locale. Auto-mirroring would leave the button pointing right while still moving
the polygon left, so they opt out with mirrorInRtl={false}. IconButton
doesn't forward that prop, so they pass a PhosphorIcon element as icon
instead — IconButton accepts PhosphorIconAsset | React.ReactElement. The
up/down arrows need nothing, since only horizontal glyphs are mirrored.

Left alone: text-align-left/right in debug-header.tsx — physical
alignment, deliberately excluded from the mirroring allowlist.

Ordering

⚠️ Blocked on Khan/wonder-blocks#3174 publishing. Two reasons:

  1. Typecheck fails until then. mirrorInRtl doesn't exist in
    wonder-blocks-icon 5.3.23, which perseus is pinned to. The red check is
    expected, not a real break.
  2. The WB bump lands in this PR. Once the major publishes, bump both catalog
    entries in pnpm-workspace.yaml (^5.3.23 peer, 5.3.23 dev) in the same
    commit. That turns the typecheck green and makes the change coherent.

Perseus declares wonder-blocks-icon as a peerDependency, so it runs against
frontend's WB version. After this publishes, frontend bumps
@khanacademy/perseus and @khanacademy/wonder-blocks-icon together in one
PR (Khan/frontend#15137) — both are exact-pinned in the same catalog there, so
they land atomically with no window where these carets render against an
un-bumped WB.

A standalone perseus bump reaching frontend before that combined PR would show
two wrong-facing scroll arrows in RTL for one cycle. Worth avoiding.

Issue: CLASS-13991

Test plan

  • pnpm jest scrollable-view — 10/10, including the three RTL cases. They
    assert on aria-label (scrollStart/scrollEnd) rather than glyph, so
    they cover behavior and stay valid.
  • pnpm tsc — currently fails on mirrorInRtl (see Ordering). Pre-existing
    @dnd-kit/react resolution errors in dnd-kit-demo.stories.tsx are
    unrelated.
  • Visual check in RTL once the WB bump is in: scroll carets point into the
    content, and the graph editor's left/right arrows still match the
    direction the polygon moves.

PhosphorIcon now mirrors directional glyphs automatically in RTL, so
picking the caret based on isRTL double-flips once Wonder Blocks is
bumped. Pass the LTR-facing carets and let mirroring handle direction.

This leaves isRTL with no readers on the file-local ScrollState, so it
is dropped; newIsRtl still drives the canScrollStart/canScrollEnd math.

Depends on the wonder-blocks-icon major bump
(Khan/wonder-blocks#3174).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

npm Snapshot

Want to try this PR's changes before it merges? Comment /snapshot below and we'll publish an npm snapshot you can install right away.

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Size Change: +10 B (0%)

Total Size: 519 kB

📦 View Changed
Filename Size Change
packages/perseus-editor/dist/es/index.js 106 kB +32 B (+0.03%)
packages/perseus/dist/es/index.js 203 kB -22 B (-0.01%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 20.6 kB
packages/keypad-context/dist/es/index.js 1 kB
packages/kmath/dist/es/index.js 6.31 kB
packages/math-input/dist/es/index.js 98.6 kB
packages/math-input/dist/es/strings.js 1.63 kB
packages/perseus-core/dist/es/index.item-splitting.js 13.1 kB
packages/perseus-core/dist/es/index.js 28.3 kB
packages/perseus-linter/dist/es/index.js 10.1 kB
packages/perseus-score/dist/es/index.js 9.86 kB
packages/perseus-utils/dist/es/index.js 403 B
packages/perseus/dist/es/strings.js 12.9 kB
packages/pure-markdown/dist/es/index.js 1.39 kB
packages/simple-markdown/dist/es/index.js 6.12 kB

compressed-size-action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant