Skip to content
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.

Sign up for GitHub

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

[Hint Mode: Start Coords] Add separate flags for graph types #1465

Merged
merged 4 commits into from
Jul 31, 2024
Merged

[Hint Mode: Start Coords] Add separate flags for graph types #1465

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f531f0b
[Hint Mode: Start Coords] Add separate flags for graph types
nishasy Jul 30, 2024
5b72fbd
reduce number of flags
nishasy Jul 31, 2024
5f3db68
make phase1 prop optional
nishasy Jul 31, 2024
5351887
Update conditional. Add tests
nishasy Jul 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/slow-taxis-give.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": patch
"@khanacademy/perseus-editor": patch
---

[Hint Mode: Start Coords] Add separate flags for graph types
17 changes: 16 additions & 1 deletion packages/perseus-editor/src/__stories__/flags-for-api-options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,24 @@ export const flags = {
linear: true,
"linear-system": true,
ray: true,
"start-coords-ui": true,

// Locked figures flags
"interactive-graph-locked-features-m2": true,
"interactive-graph-locked-features-m2b": true,

// Start coords UI flags
"start-coords-ui": {
angle: false,
segment: true,
circle: true,
quadratic: false,
sinusoid: false,
polygon: false,
linear: true,
"linear-system": true,
ray: true,
point: false,
},
},
} satisfies APIOptions["flags"];

Expand Down
2 changes: 0 additions & 2 deletions packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ export const MafsWithLockedFiguresCurrent = (): React.ReactElement => {
flags: {
mafs: {
...flags.mafs,
"start-coords-ui": false,
"interactive-graph-locked-features-m2": false,
"interactive-graph-locked-features-m2b": false,
},
Expand All @@ -149,7 +148,6 @@ export const MafsWithLockedFiguresM2Flag = (): React.ReactElement => {
flags: {
mafs: {
...flags.mafs,
"start-coords-ui": false,
"interactive-graph-locked-features-m2": true,
"interactive-graph-locked-features-m2b": false,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus-editor/src/widgets/interactive-graph-editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ class InteractiveGraphEditor extends React.Component<Props> {
</LabeledRow>
)}
{this.props.graph?.type &&
this.props.apiOptions?.flags?.mafs?.["start-coords-ui"] && (
(this.props.apiOptions?.flags?.mafs?.["start-coords-ui"][this.props.graph.type]) && (
Copy link
Member

@benchristel benchristel Jul 31, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be

Suggested change
(this.props.apiOptions?.flags?.mafs?.["start-coords-ui"][this.props.graph.type]) && (
(this.props.apiOptions?.flags?.mafs?.["start-coords-ui"]?.[this.props.graph.type]) && (

or it will throw an error when mafs["start-coords-ui"] is undefined.

EDIT: oh, I see that ["start-coords-ui"] is a required property. I guess this is okay, then, because TypeScript will make sure start-coords-ui is always an object when the flags are set in webapp.

<StartCoordsSettings
{...this.props.graph}
range={this.props.range}
Expand Down
9 changes: 1 addition & 8 deletions packages/perseus/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,6 @@ export const InteractiveGraphLockedFeaturesFlags = [
"interactive-graph-locked-features-m2b",
] as const;

export const InteractiveGraphEditorFlags = [
/**
* Enables the UI for setting the start coordinates of a graph.
*/
"start-coords-ui",
] as const;

/**
* APIOptions provides different ways to customize the behaviour of Perseus.
*
Expand Down Expand Up @@ -304,7 +297,7 @@ export type APIOptions = Readonly<{
| ({[Key in (typeof MafsGraphTypeFlags)[number]]?: boolean} & {
[Key in (typeof InteractiveGraphLockedFeaturesFlags)[number]]?: boolean;
} & {
[Key in (typeof InteractiveGraphEditorFlags)[number]]?: boolean;
"start-coords-ui": {[Key in (typeof MafsGraphTypeFlags)[number]]?: boolean};
});
};
/**
Expand Down
Loading