-
Notifications
You must be signed in to change notification settings - Fork 350
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
[Interactive Graph] New Axes type for graph markings. #2053
base: main
Are you sure you want to change the base?
Conversation
Size Change: +49 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (078eeee) and published it to npm. You Example: yarn add @khanacademy/perseus@PR2053 If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR2053 |
/** | ||
* The type of markings to display on the graph. | ||
* - axes: shows the axes without the gride lines | ||
* - graph: shows the axes and the grid lines | ||
* - grid: shows only the grid lines | ||
* - none: shows no markings | ||
*/ |
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 the possible values are not actually defined on this line anymore, it seems like this comment might drift out of sync with the code. How would you feel about documenting the different values where MarkingsType
is defined (in perseus-types.ts)?
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.
The code looks good! I think we should probably have a visual test for the axes
option, in interactive-graph-regression.stories.ts
.
Writing that test will probably involve updating interactive-graph-question-builder.ts
as well, which currently has hardcoded "grid" | "graph" | "none"
.
Summary:
At request from content creators they would like to have a new marking type for graph that will show the axes without requiring a full grid.
Issue: LEMS-2713
Test plan:
Go to: http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-linear or any other graph editor.
Select
Axes
option and notice new axes graph markings.