-
Notifications
You must be signed in to change notification settings - Fork 357
[Interactive Graph] Add showAxisArrows
field to Interactive Graph widget
#2777
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.
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
base: main
Are you sure you want to change the base?
Conversation
…`boundedSides` field Working on adding a way for content authors to specify which axis arrows to remove for graphs that need to be bounded in some direction. This can be any combination of x min, x max, y min, and y max directions. In this PR, I'm adding a `boundedSides` field to the Interactive Graph schema, and I'm connecting it to the widget. The editor updates will be in a following PR. Issue: https://khanacademy.atlassian.net/browse/LEMS-3214 Test plan: `pnpm jest packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts` Check this PR for the new regression tests, check future PRs for regression failures Storybook - http://localhost:6006/?path=/story/perseus-widgets-interactive-graph-visual-regression-tests--bounded-x-min - http://localhost:6006/?path=/story/perseus-widgets-interactive-graph-visual-regression-tests--bounded-x-max - http://localhost:6006/?path=/story/perseus-widgets-interactive-graph-visual-regression-tests--bounded-y-min - http://localhost:6006/?path=/story/perseus-widgets-interactive-graph-visual-regression-tests--bounded-y-max - http://localhost:6006/?path=/story/perseus-widgets-interactive-graph-visual-regression-tests--bounded-all-sides
🗄️ Schema Change: Changes Detected
|
🛠️ Item Splitting: Changes Detected
|
Size Change: +189 B (+0.04%) Total Size: 496 kB
ℹ️ View Unchanged
|
…Arrows` field to Interactive Graph widget
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file has the main logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good! I'll hold off on approval until you mark it "ready for review" though.
} = useGraphConfig(); | ||
|
||
const axisColor = "var(--mafs-fg)"; | ||
|
||
// Arrow defaults to true if not specified. | ||
const arrows: ShowAxisArrows = | ||
showAxisArrows === undefined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we default showAxisArrows
in the parser, why would it be undefined
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, that's left over from before.
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (7b43166) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR2777 If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR2777 If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR2777 |
Summary:
Working on adding a way for content authors to specify which axis arrows to remove
for graphs that need to be bounded in some direction. This can be any combination
of x min, x max, y min, and y max directions.
In this PR, I'm adding a
showAxisArrows
field to the Interactive Graph schema, andI'm connecting it to the widget. The editor updates will be in a following PR.
Note:
showAxisArrows
towidgets.go
Issue: https://khanacademy.atlassian.net/browse/LEMS-3214
Screenshots:
Test plan:
pnpm jest packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts
pnpm jest packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx
Check this PR for the new regression tests, check future PRs for regression failures
Storybook