provide keypad for expression story #442

handeyeco · 2023-03-27T21:29:43Z

Summary:

I think I'm starting to understand the mobile web keypad:

We have a Keypad context (Provider/Consumer)
We have the actual Keypad component (math-input/src/components/keypad-container.js)
We have the MathInput component

Then we shuffle it all around chaotically. Something like:

function App() {
  const [keypadElement, setKeypadElement] = useState()
  return (
    <KeypadContext.Provider value={{ keypadElement, setKeypadElement }}>
      <KeypadInput keypadElement={keypadElement} />
      <KeypadContext.Consumer>
        {({keypadElement, setKeypadElement}) => (
          <Keypad onElementMounted={setKeypadElement} />
        )}
      </KeypadContext.Consumer>
    </KeypadContext.Provider>
  )
}

Why all the hoop jumping? Still don't know why. Seems like MathInput could just render the input and the keypad without the need for Context, but I'm sure there was a good reason.

ANYWAY

This explains why I wasn't able to see the Keypad on the Expression widget in Storybook: it wasn't using Context. This PR creates a shared component for providing context (with the on-screen-keypad story, which might be obsolete now).

Unfortunately all this indirection kills my dream of having MathInput manage how we render the keypad; I think it's going to have to happen in Perseus unless we really want to overhaul things.

Screen.Recording.2023-03-27.at.4.31.13.PM.mov

changeset-bot · 2023-03-27T21:29:46Z

🦋 Changeset detected

Latest commit: 511b0d2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-03-27T21:33:41Z

Size Change: 0 B

Total Size: 652 kB

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38 kB
`packages/kmath/dist/es/index.js`	4.13 kB
`packages/math-input/dist/es/index.js`	60.3 kB
`packages/perseus-editor/dist/es/index.js`	114 kB
`packages/perseus-error/dist/es/index.js`	705 B
`packages/perseus-linter/dist/es/index.js`	21.2 kB
`packages/perseus/dist/es/index.js`	398 kB
`packages/pure-markdown/dist/es/index.js`	3.65 kB
`packages/simple-markdown/dist/es/index.js`	12.6 kB

_{compressed-size-action}

coveralls · 2023-03-27T21:45:46Z

Coverage: 43.018% (+0.0005%) from 43.017% when pulling 511b0d2 on provide-expression-story into ec4389e on main.

github-actions · 2023-03-27T21:55:07Z

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (c4805c6) and published it to npm. You
can install it using the tag PR442.

Example:

yarn add @khanacademy/perseus@PR442

handeyeco · 2023-03-28T15:11:37Z

packages/perseus/src/widgets/__stories__/expression.stories.jsx

+            <KeypadContext.Consumer>
+                {({keypadElement, setRenderer, scrollableElement}) => {
+                    return (
+                        <ItemRendererWithDebugUI


I switched to ItemRendererWithDebugUI because ItemRenderer has keypadElement={this.keypadElement()} when rendering Renderer whereas RendererWithDebugUI does not - which was causing errors when trying to open the keypad.

I imagine I could have added it to RendererWithDebugUI, but I'm not totally sure I understand why we have both ItemRendererWithDebugUI and RendererWithDebugUI. Figured it was best to let sleeping dogs lie.

The two "RendererWithDebugUI" helpers render different components. In Perseus, we have ItemRenderer and Renderer. The former is high-level and accepts a bigger blob of JSON, the Renderer is our core, lower-level component that renders a single PerseusRenderer object (the thing that looks like {content: string, widgets: {}, images: {}}.

handeyeco · 2023-03-28T15:45:36Z

packages/perseus/src/widgets/__stories__/test-keypad-context-wrapper.jsx

@@ -4,60 +4,11 @@ import {View} from "@khanacademy/wonder-blocks-core";
 import {StyleSheet} from "aphrodite";
 import * as React from "react";

-import {ItemRenderer} from "../../index.js";


Originally I split this logic out so both on-screen-keypad.stories.jsx and expression.stories.jsx could use the same keypad provider. However (IMO) on-screen-keypad.stories.jsx was really just a very specific expression story, so I ended up just strengthening up expression.stories.jsx and getting rid of on-screen-keypad.stories.jsx.

Keeping this split out logic separate though, because (I think) we want the keypad in lots of components. Also I didn't want to clutter expression.stories.jsx.

handeyeco · 2023-03-28T15:46:44Z

packages/perseus/src/widgets/__testdata__/expression_testdata.js

 const createItemJson = (
    widgetOptions: PerseusExpressionWidgetOptions,
    version: Version,
-): Item => {
+): PerseusItem => {


Using the real PerseusItem type instead of the fake Item. That required adding a few fields to the object.

khan-actions-bot · 2023-03-28T15:47:47Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/thick-islands-kick.md, packages/perseus/src/widgets/__stories__/expression.stories.tsx, packages/perseus/src/widgets/__stories__/test-keypad-context-wrapper.tsx, packages/perseus/src/widgets/__testdata__/expression.testdata.ts

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

jeremywiebe

Cool. Glad you feel you're starting to understand all this Math Input stuff... it's still very much a mystery to me! :)

jeremywiebe · 2023-03-30T16:47:34Z

packages/perseus/src/widgets/__stories__/expression.stories.jsx

+            <KeypadContext.Consumer>
+                {({keypadElement, setRenderer, scrollableElement}) => {
+                    return (
+                        <ItemRendererWithDebugUI


The two "RendererWithDebugUI" helpers render different components. In Perseus, we have ItemRenderer and Renderer. The former is high-level and accepts a bigger blob of JSON, the Renderer is our core, lower-level component that renders a single PerseusRenderer object (the thing that looks like {content: string, widgets: {}, images: {}}.

provide keypad for expression story

4918919

handeyeco self-assigned this Mar 27, 2023

add changeset

4e3456a

handeyeco commented Mar 28, 2023

View reviewed changes

handeyeco added 2 commits March 28, 2023 10:33

clean up code

a24ca69

more cleanup

e427a87

handeyeco commented Mar 28, 2023

View reviewed changes

handeyeco marked this pull request as ready for review March 28, 2023 15:47

handeyeco requested a review from a team March 28, 2023 15:49

add direct expression render to Storybook

e58a14a

jeremywiebe approved these changes Mar 30, 2023

View reviewed changes

handeyeco added 2 commits April 3, 2023 11:18

fix conflicts I hope

55e9b0e

switch new test file to TS

511b0d2

handeyeco merged commit f1ffa47 into main Apr 4, 2023

handeyeco deleted the provide-expression-story branch April 4, 2023 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provide keypad for expression story #442

provide keypad for expression story #442

handeyeco commented Mar 27, 2023 •

edited

Loading

changeset-bot bot commented Mar 27, 2023 •

edited

Loading

github-actions bot commented Mar 27, 2023 •

edited

Loading

coveralls commented Mar 27, 2023 •

edited

Loading

github-actions bot commented Mar 27, 2023 •

edited

Loading

handeyeco Mar 28, 2023

jeremywiebe Mar 30, 2023

handeyeco Mar 28, 2023

handeyeco Mar 28, 2023

khan-actions-bot commented Mar 28, 2023 •

edited

Loading

jeremywiebe left a comment

jeremywiebe Mar 30, 2023

provide keypad for expression story #442

provide keypad for expression story #442

Conversation

handeyeco commented Mar 27, 2023 • edited Loading

Summary:

changeset-bot bot commented Mar 27, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Mar 27, 2023 • edited Loading

coveralls commented Mar 27, 2023 • edited Loading

github-actions bot commented Mar 27, 2023 • edited Loading

npm Snapshot: Published

handeyeco Mar 28, 2023

Choose a reason for hiding this comment

jeremywiebe Mar 30, 2023

Choose a reason for hiding this comment

handeyeco Mar 28, 2023

Choose a reason for hiding this comment

handeyeco Mar 28, 2023

Choose a reason for hiding this comment

khan-actions-bot commented Mar 28, 2023 • edited Loading

Gerald

jeremywiebe left a comment

Choose a reason for hiding this comment

jeremywiebe Mar 30, 2023

Choose a reason for hiding this comment

handeyeco commented Mar 27, 2023 •

edited

Loading

changeset-bot bot commented Mar 27, 2023 •

edited

Loading

github-actions bot commented Mar 27, 2023 •

edited

Loading

coveralls commented Mar 27, 2023 •

edited

Loading

github-actions bot commented Mar 27, 2023 •

edited

Loading

khan-actions-bot commented Mar 28, 2023 •

edited

Loading