Add onJsonModeChange callback to EditorPage - #4188
Conversation
…`onJsonModeChange` prop to inform clients of when JSON mode is toggled. The `jsonMode` parameter to `onChange` is deprecated.
npm SnapshotWant to try this PR's changes before it merges? Comment |
|
Size Change: +20 B (0%) Total Size: 513 kB 📦 View Changed
ℹ️ View Unchanged
|
| const newJsonMode = !this.props.jsonMode; | ||
| this.props.onChange({ | ||
| jsonMode: !this.props.jsonMode, | ||
| jsonMode: newJsonMode, |
There was a problem hiding this comment.
Why not just rip the bandaid and remove this?
There was a problem hiding this comment.
I don't want the next person to release Perseus to be blocked by having to update all onChange callbacks.
| "@khanacademy/perseus-editor": minor | ||
| --- | ||
|
|
||
| `EditorPage` now accepts an `onJsonModeChange` prop to inform clients of when JSON mode is toggled. The `jsonMode` parameter to `onChange` is deprecated. |
There was a problem hiding this comment.
I wonder why we have two ways to switch to JSON mode: a switch outside of Perseus and a checkbox inside of Perseus?
There was a problem hiding this comment.
Because originally the toggle was inside EditorPage and its availability was controlled by an APIOption (I can't remember what it was)... and then someone wanted it outside so we have both.
When we break the EditorPage down into smaller components, Perseus will no longer manage switching between views, we'll just export the editor and a JSON viewer/editor and consumers can do what they want.
3 participants
Summary:
The plan is to remove
jsonModefromonChange, soonChangecan just accepta
PerseusItem.Issue: LEMS-4570
Test plan:
CI checks should pass.