Addition of new alert when json is converted. #1859

SonicScrewdriver · 2024-11-14T20:59:55Z

Summary:

This PR adds an alert banner to the editors to notify Content Editors when pre-existing content has been converted into modern widgets. (ie Input Number has become Numeric Input)

Screenshot:

Test plan:

Manual testing

khan-actions-bot · 2024-11-14T21:00:11Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/many-boxes-wave.md, packages/perseus-editor/src/article-editor.tsx, packages/perseus-editor/src/editor-page.tsx, packages/perseus-editor/src/util/deprecated-widgets/modernize-widgets-utils.ts

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

SonicScrewdriver · 2024-11-14T21:02:15Z

packages/perseus-editor/src/util/deprecated-widgets/modernize-widgets-utils.ts

-    if (!conversionRequired(json)) {
-        return json;
-    }
-


Don't need to check here any more, as we're doing it directly in the editors. This allows us to know when the json has been updated! :)

github-actions · 2024-11-14T21:03:54Z

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1859

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1859

github-actions · 2024-11-14T21:03:59Z

Size Change: +350 B (+0.04%)

Total Size: 872 kB

Filename	Size	Change
`packages/perseus-editor/dist/es/index.js`	285 kB	+350 B (+0.12%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.9 kB
`packages/keypad-context/dist/es/index.js`	760 B
`packages/kmath/dist/es/index.js`	4.27 kB
`packages/math-input/dist/es/index.js`	77.8 kB
`packages/math-input/dist/es/strings.js`	1.79 kB
`packages/perseus-core/dist/es/index.js`	1.48 kB
`packages/perseus-linter/dist/es/index.js`	22.2 kB
`packages/perseus/dist/es/index.js`	420 kB
`packages/perseus/dist/es/strings.js`	3.54 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.4 kB

_{compressed-size-action}

SonicScrewdriver · 2024-11-14T21:07:20Z

packages/perseus-editor/src/article-editor.tsx

+                ? props.json.map(convertDeprecatedWidgets)
+                : convertDeprecatedWidgets(props.json as PerseusRenderer);
+        }
+


I felt it was better to keep this a little verbose to help with legibility

jeremywiebe

Makes sense. :)

jeremywiebe · 2024-11-14T21:48:54Z

packages/perseus-editor/src/article-editor.tsx

+        const conversionWarningRequired = conversionRequired(
+            props.json as PerseusRenderer,
+        );


This is probably not be safe. I'm pretty sure there are places in webapp where it passes in ReadonlyArray<PerseusRenderer> (essentially the PeprseusArticle type). I think it can be fixed fairly easily though with:

Suggested change

const conversionWarningRequired = conversionRequired(

props.json as PerseusRenderer,

);

const conversionWarningRequired =

props.json instanceof Array

? props.json

.map(conversionRequired)

.reduce((p, v) => p && v, false)

: conversionRequired(props.json);

Oh great catch, thank you! I forgot to check for the sections as part of the conversionRequired check

jeremywiebe · 2024-11-14T21:52:34Z

packages/perseus-editor/src/article-editor.tsx

+
+        // Convert the json if needed
+        if (conversionWarningRequired) {
+            json = Array.isArray(props.json)


If you use props.json instanceof Array. That will still do array checking but properly narrow props.json so you don't need the as.

jeremywiebe · 2024-11-14T21:53:47Z

packages/perseus-editor/src/article-editor.tsx

+                {this.state.conversionWarningRequired && (
+                    <div style={{marginBottom: 10}}>
+                        <Banner
+                            text="Pre-existing Input Number Widgets have been converted to Numeric Inputs. Please review the changes before publishing."


nit: "Pre-existing" threw me... where did they exist before? :)

Perhaps "Deprecated Input Number widgets were found in this article! They have been automatically upgraded to Numeric Inputs. Please review the changes before publishing."

jeremywiebe · 2024-11-14T21:55:00Z

packages/perseus-editor/src/editor-page.tsx

+                {this.state.conversionWarningRequired && (
+                    <div style={{marginBottom: 10}}>
+                        <Banner
+                            text="Pre-existing Input Number Widgets have been converted to Numeric Inputs. Please review the changes before publishing."


Same nit about "pre-existing". 😄

@SonicScrewdriver

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/[email protected] ### Minor Changes - [#1731](#1731) [`27126aa00`](27126aa) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Conversion of Input Number to Numeric Input ### Patch Changes - [#1846](#1846) [`8eb1ff5d1`](8eb1ff5) Thanks [@benchristel](https://github.com/benchristel)! - Internal: add widget parsers for ADR 773. - [#1839](#1839) [`150888870`](1508888) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Util function to generate spoken math + use it within Locked Point aria labels ## @khanacademy/[email protected] ### Minor Changes - [#1859](#1859) [`dcf1fbe35`](dcf1fbe) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Addition of a new alert for the content editors when Input numbers are converted to Numeric Inputs - [#1731](#1731) [`27126aa00`](27126aa) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Conversion of Input Number to Numeric Input ### Patch Changes - [#1839](#1839) [`150888870`](1508888) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Figure Labels] Util function to generate spoken math + use it within Locked Point aria labels - Updated dependencies \[[`8eb1ff5d1`](8eb1ff5), [`150888870`](1508888), [`27126aa00`](27126aa)]: - @khanacademy/[email protected]

This reverts commit dcf1fbe.

@SonicScrewdriver

… logic (#1905) ## Summary: In order to unblock Perseus, we are reverting the following commits: - [x] [#1888](#1888) [d0e7a03](d0e7a03) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Fixing issue with Input Numbers that have a value of 0 - [x] [#1884](#1884) [b4cf444](b4cf444) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Ensuring UserInput and Rubric widget keys match for edge cases - [x] [#1879](#1879) [04d6e60](04d6e60) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Fixing conflicts that arose from scoring and widget conversion efforts - [x] [#1753](#1753) [c1ba55f](c1ba55f) Thanks [@handeyeco](https://github.com/handeyeco)! - Change ServerItemRenderer scoring APIs to externalize scoring - [x] [#1866](#1866) [94eba15](94eba15) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Fixing a regression and a bug in the Input Conversion Logic - [x] [#1859](#1859) [dcf1fbe](dcf1fbe) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Addition of a new alert for the content editors when Input numbers are converted to Numeric Inputs - [x] [#1731](#1731) [27126aa](27126aa) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Conversion of Input Number to Numeric Input ## Test plan: - manual testing Author: SonicScrewdriver Reviewers: catandthemachines, #perseus Required Reviewers: Approved By: catandthemachines Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald, 🚫 Publish npm snapshot (ubuntu-latest, 20.x), 🚫 Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), 🚫 Check for .changeset entries for all changed files (ubuntu-latest, 20.x), 🚫 Check builds for changes in size (ubuntu-latest, 20.x), 🚫 Cypress (ubuntu-latest, 20.x), ✅ gerald, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1905

@SonicScrewdriver

… logic (#1905) ## Summary: In order to unblock Perseus, we are reverting the following commits: - [x] [#1888](#1888) [d0e7a03](d0e7a03) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Fixing issue with Input Numbers that have a value of 0 - [x] [#1884](#1884) [b4cf444](b4cf444) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Ensuring UserInput and Rubric widget keys match for edge cases - [x] [#1879](#1879) [04d6e60](04d6e60) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Fixing conflicts that arose from scoring and widget conversion efforts - [x] [#1753](#1753) [c1ba55f](c1ba55f) Thanks [@handeyeco](https://github.com/handeyeco)! - Change ServerItemRenderer scoring APIs to externalize scoring - [x] [#1866](#1866) [94eba15](94eba15) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Fixing a regression and a bug in the Input Conversion Logic - [x] [#1859](#1859) [dcf1fbe](dcf1fbe) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Addition of a new alert for the content editors when Input numbers are converted to Numeric Inputs - [x] [#1731](#1731) [27126aa](27126aa) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Conversion of Input Number to Numeric Input ## Test plan: - manual testing Author: SonicScrewdriver Reviewers: catandthemachines, #perseus Required Reviewers: Approved By: catandthemachines Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald, 🚫 Publish npm snapshot (ubuntu-latest, 20.x), 🚫 Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), 🚫 Check for .changeset entries for all changed files (ubuntu-latest, 20.x), 🚫 Check builds for changes in size (ubuntu-latest, 20.x), 🚫 Cypress (ubuntu-latest, 20.x), ✅ gerald, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1905

Addition of new alert when json is converted.

a4d0dcf

SonicScrewdriver self-assigned this Nov 14, 2024

khan-actions-bot requested a review from a team November 14, 2024 21:00

SonicScrewdriver commented Nov 14, 2024

View reviewed changes

changeset

18880b5

SonicScrewdriver requested a review from mark-fitzgerald November 14, 2024 21:06

SonicScrewdriver commented Nov 14, 2024

View reviewed changes

mark-fitzgerald approved these changes Nov 14, 2024

View reviewed changes

jeremywiebe approved these changes Nov 14, 2024

View reviewed changes

Addressing PR Feedback

502f602

SonicScrewdriver merged commit dcf1fbe into main Nov 14, 2024
8 of 9 checks passed

SonicScrewdriver deleted the numeric-alert branch November 14, 2024 22:51

khan-actions-bot mentioned this pull request Nov 14, 2024

Version Packages #1857

Merged

SonicScrewdriver added a commit that referenced this pull request Nov 22, 2024

Revert "Addition of new alert when json is converted. (#1859)"

fbcea00

This reverts commit dcf1fbe.

SonicScrewdriver mentioned this pull request Nov 22, 2024

Reverting all Server Side Scoring and Input Number conversion Project logic #1905

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addition of new alert when json is converted. #1859

Addition of new alert when json is converted. #1859

SonicScrewdriver commented Nov 14, 2024 •

edited

Loading

khan-actions-bot commented Nov 14, 2024 •

edited

Loading

SonicScrewdriver Nov 14, 2024

github-actions bot commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 14, 2024 •

edited

Loading

SonicScrewdriver Nov 14, 2024 •

edited

Loading

jeremywiebe left a comment

jeremywiebe Nov 14, 2024

SonicScrewdriver Nov 14, 2024

jeremywiebe Nov 14, 2024

jeremywiebe Nov 14, 2024

jeremywiebe Nov 14, 2024

Addition of new alert when json is converted. #1859

Addition of new alert when json is converted. #1859

Conversation

SonicScrewdriver commented Nov 14, 2024 • edited Loading

Summary:

Screenshot:

Test plan:

khan-actions-bot commented Nov 14, 2024 • edited Loading

Gerald

SonicScrewdriver Nov 14, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 14, 2024 • edited Loading

npm Snapshot: Published

github-actions bot commented Nov 14, 2024 • edited Loading

SonicScrewdriver Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

jeremywiebe left a comment

Choose a reason for hiding this comment

jeremywiebe Nov 14, 2024

Choose a reason for hiding this comment

SonicScrewdriver Nov 14, 2024

Choose a reason for hiding this comment

jeremywiebe Nov 14, 2024

Choose a reason for hiding this comment

jeremywiebe Nov 14, 2024

Choose a reason for hiding this comment

jeremywiebe Nov 14, 2024

Choose a reason for hiding this comment

SonicScrewdriver commented Nov 14, 2024 •

edited

Loading

khan-actions-bot commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 14, 2024 •

edited

Loading

github-actions bot commented Nov 14, 2024 •

edited

Loading

SonicScrewdriver Nov 14, 2024 •

edited

Loading