Spike: swap input-number with numeric-input 😈 #418
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 63e4cac The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (d3ec356) and published it to npm. You Example: yarn add @khanacademy/perseus@PR418 |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## feature/remove-input-number #418 +/- ##
================================================================
- Coverage 65.49% 28.48% -37.01%
================================================================
Files 481 304 -177
Lines 103927 25958 -77969
Branches 5620 6271 +651
================================================================
- Hits 68064 7394 -60670
+ Misses 35863 18564 -17299
... and 484 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
Size Change: -121 B (0%) Total Size: 635 kB
ℹ️ View Unchanged
|
## Summary: This is my first wave of cleanup for expression/math-input. Should be all dev-facing. - Flow types instead of prop types - Removed a `react/sort-comp` exception, then reorganized the component to pass - Renamed a file to make it clearer that it's for testing only - Removed some old CSS vender prefixes Issue: LC-618 ## Test plan: - Nothing should change Author: handeyeco Reviewers: handeyeco, jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ✅ codecov/project, ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ gerald Pull Request URL: #414
## Summary: Just a very quick pass to get _some_ Flow typing in `math-input`. I would happily continue, but I'm also supposed to be shipping updates to users too. Issue: LC-618 ## Test plan: - Nothing should change for users, just new types Author: handeyeco Reviewers: handeyeco, jeremywiebe Required Reviewers: Approved By: jeremywiebe, jeremywiebe Checks: ✅ codecov/project, ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ gerald, ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x) Pull Request URL: #415
## Summary: This is just a quick pass to remove some CSS vendor prefixes. Some notes: - Less files should be going through the autoprefixer, so Less files should basically never have prefixes - I tried my best to cross reference caniuse.com and our updated supported browsers doc - Things that were doing complex logic around styles I left alone - Prefixes that were particularly obscure I also left alone ## Test plan: Nothing should change Author: handeyeco Reviewers: jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ❌ codecov/project, ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ gerald Pull Request URL: #416
## Summary: I found a misnamed component Author: handeyeco Reviewers: jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ✅ codecov/project, ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ gerald, ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ gerald, ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ gerald, ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x) Pull Request URL: #417
## Summary: Now that wonder-blocks has been updated to use TS and the Flow types we're generating for those packages are working in webapp we need to update Perseus to use those same modules. This avoids shipping more code that we need and prevents subtle issues that can occur when using different version of the same packages via the same import name. The major version number bumps in the wonder-blocks dependencies are to communicate changes in the flow types. The only functional change is a minor change to link. There was an issue with link the last time we tried to update it where it wasn't respecting the surrounding font size. This version of link fixes that. Issue: FEI-5042 ## Test plan: - yarn flow - let CI run Author: kevinbarabash Reviewers: kevinbarabash, jeresig, handeyeco Required Reviewers: Approved By: jeresig, handeyeco Checks: ✅ codecov/project, ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ❌ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ❌ Cypress Coverage (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ gerald, ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ❌ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ gerald Pull Request URL: #419
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/perseus@2.1.0 ### Minor Changes - 2c843b3: Update to use wonder-blocks deps after migrating wonder-blocks to TS ### Patch Changes - Updated dependencies [2c843b3] - @khanacademy/math-input@0.6.8 ## @khanacademy/perseus-editor@1.1.0 ### Minor Changes - 2c843b3: Update to use wonder-blocks deps after migrating wonder-blocks to TS ### Patch Changes - Updated dependencies [2c843b3] - @khanacademy/perseus@2.1.0 ## @khanacademy/math-input@0.6.8 ### Patch Changes - 2c843b3: Update to use wonder-blocks deps after migrating wonder-blocks to TS Author: khan-actions-bot Reviewers: kevinbarabash Required Reviewers: Approved By: kevinbarabash Checks: ✅ codecov/project, ❌ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ❌ Cypress Coverage (ubuntu-latest, 16.x), ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ gerald, ✅ Lint, Flow, and Test (ubuntu-latest, 16.x) Pull Request URL: #420
## Summary: I want to see if Coveralls is more reliable than Codecov. Also, Coveralls is cheaper for use with private repos so if it works well we'll probably want to use it for that use case as well. Issue: None ## Test plan: - let CI run, see what the output looks like Author: kevinbarabash Reviewers: jaredly, jeremywiebe, kevinbarabash Required Reviewers: Approved By: jaredly, jeremywiebe Checks: ✅ finish_coverage, ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ gerald, ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x) Pull Request URL: #370
## Summary: We've seen Github action failures in the "Publish npm snapshot" job. This is because the npm auth info has not been provided (this is done automatically by the changesets Github Action, but we aren't using it here because we're publishing a snapshot. This PR mimics what the changesets Github Action [does](https://github.com/changesets/action/blob/8c3f5f5637a95a2327e78d5dabcf357978aedcbb/src/index.ts#L58..L85) by creating a `.npmrc` file with the credentials in it. Issue: "none" ## Test plan: Create a new PR after landing this one. Ensure that the "Publish npm snapshot" job succeeds. Author: jeremywiebe Reviewers: jeremywiebe, handeyeco Required Reviewers: Approved By: handeyeco Checks: ✅ finish_coverage, ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ gerald, ✅ Check builds for changes in size (ubuntu-latest, 16.x) Pull Request URL: #422
…hes (#425) * Upload coverage to Coveralls when running on 'main' and feature branches * add empty changeset file * Update .github/workflows/node-ci-main.yml Co-authored-by: Jeremy Wiebe <jeremy@khanacademy.org> * Specify a version for the action, add fix options in PR workflow * update version of coveralls in node-ci as well --------- Co-authored-by: Jeremy Wiebe <jeremy@khanacademy.org>
jeremywiebe
force-pushed
the
swap-in-numeric-input
branch
from
4747148
to
53bfdda
Compare
* Try Coveralls v1 * add empty changeset file
## Summary: I ran across `staticRender` in `ApiOptions` while working on Expression. I couldn't figure out what it was for and, after digging in a bit, realized it was meant to be depreciated years ago. I couldn't find anything that was setting `staticRender` to `true` (or anything that could set it). See this rambling: https://khanacademy.slack.com/archives/C01AZ9H8TTQ/p1679519079246219 A highlight from 2016: > At this point, it doesn't feel like it's worth changing, **since we'll hopefully remove staticRender as soon as we're using the customKeypad everywhere.** ## Test plan: - Nothing should change in webapp/mobile Author: handeyeco Reviewers: jeremywiebe, handeyeco Required Reviewers: Approved By: jeremywiebe Checks: ✅ finish_coverage, ✅ Publish npm snapshot (ubuntu-latest, 16.x), ✅ Extract i18n strings (ubuntu-latest, 16.x), ✅ Cypress Coverage (ubuntu-latest, 16.x), ✅ Check builds for changes in size (ubuntu-latest, 16.x), ✅ Jest Coverage (ubuntu-latest, 16.x), ✅ Lint, Flow, and Test (ubuntu-latest, 16.x), ✅ Check for .changeset file (ubuntu-latest, 16.x), ✅ gerald Pull Request URL: #421
Version Packages
jeremywiebe
force-pushed
the
swap-in-numeric-input
branch
from
53bfdda
to
63e4cac
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Issue: LC-532
Test plan: