[CLASS-13991] Stop swapping scroll caret icons on isRTL - #4104
Draft
maddy531 wants to merge 3 commits into
Draft
Conversation
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>
Contributor
npm SnapshotWant to try this PR's changes before it merges? Comment |
4 tasks
Contributor
|
Size Change: +10 B (0%) Total Size: 519 kB 📦 View Changed
ℹ️ View Unchanged
|
…on mirror them in RTL, opting the graph-editor's absolute-axis arrows out.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PhosphorIconmirrors directional glyphs automatically in RTL once thewonder-blocks-iconmajor lands (Khan/wonder-blocks#3174). This adapts perseusto that, in the two places it matters:
scrollable-view.tsx— picking the caret fromisRTLdouble-flips againstauto-mirroring, so it now passes the LTR-facing carets and lets mirroring handle
direction. That leaves
isRTLwith no readers on the file-localScrollStateinterface, so it's dropped;
newIsRtlstill drives thecanScrollStart/canScrollEndmath and stays.locked-polygon-settings.tsx— the "move polygon left/right" arrows pointalong 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}.IconButtondoesn't forward that prop, so they pass a
PhosphorIconelement asiconinstead —
IconButtonacceptsPhosphorIconAsset | React.ReactElement. Theup/down arrows need nothing, since only horizontal glyphs are mirrored.
Left alone:
text-align-left/rightindebug-header.tsx— physicalalignment, deliberately excluded from the mirroring allowlist.
Ordering
mirrorInRtldoesn't exist inwonder-blocks-icon5.3.23, which perseus is pinned to. The red check isexpected, not a real break.
entries in
pnpm-workspace.yaml(^5.3.23peer,5.3.23dev) in the samecommit. That turns the typecheck green and makes the change coherent.
Perseus declares
wonder-blocks-iconas apeerDependency, so it runs againstfrontend's WB version. After this publishes, frontend bumps
@khanacademy/perseusand@khanacademy/wonder-blocks-icontogether in onePR (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. Theyassert on
aria-label(scrollStart/scrollEnd) rather than glyph, sothey cover behavior and stay valid.
pnpm tsc— currently fails onmirrorInRtl(see Ordering). Pre-existing@dnd-kit/reactresolution errors indnd-kit-demo.stories.tsxareunrelated.
content, and the graph editor's left/right arrows still match the
direction the polygon moves.