Conversation
npm SnapshotWant to try this PR's changes before it merges? Comment |
|
Size Change: +139 B (+0.03%) Total Size: 519 kB 📦 View Changed
ℹ️ View Unchanged
|
| function findChildOrAdd(elem: HTMLElement, className: string): HTMLElement { | ||
| const child = elem.querySelector<HTMLElement>("." + className); | ||
| if (child) { | ||
| return child; |
There was a problem hiding this comment.
This confused me at first, but it just flipped the logic: instead of exiting early when there is no child, it exits early when there is a child.
jeremywiebe
left a comment
jeremywiebe
left a comment
There was a problem hiding this comment.
Pretty cool! Thanks
| textarea.focus(); | ||
| textarea.setSelectionRange(1, 1); | ||
| // NOTE: we use `fireEvent` because userEvent.paste doesn't go | ||
| // through the jQuery-bound paste listener, but fireEvent does. |
There was a problem hiding this comment.
Well this is a nice side-benefit that we can switch this!
| "katex": "0.11.1", | ||
| "mafs": "^0.19.0", | ||
| "tiny-invariant": "catalog:prodDeps", | ||
| "jquery": "catalog:prodDeps" |
There was a problem hiding this comment.
Does this really catch all remaining uses of jQuery in this editor package? Awesome!
There was a problem hiding this comment.
What do you think about converting this to a modern React ref, which would then relieve us of needing the ReactDOM.findDOMNode(this.refs.graphieDiv); above?
| // WB Themeing support comes via CSS variables that are imported from | ||
| // this tokens package and are included from | ||
| // .storybook/styles/shared.css | ||
| "@khanacademy/wonder-blocks-tokens", |
There was a problem hiding this comment.
Is this related to the jQuery removal?
There was a problem hiding this comment.
I don't know what was up. My original commit failed the Knip check: https://github.com/Khan/perseus/actions/runs/32293020676/job/96197933830
Removing that line passed the Knip check. I can try to add it back if you think we need it.
There was a problem hiding this comment.
Nah, we can remove it. We have imports of this package in perseus-editor, so I think the comment is invalid anyways.
There was a problem hiding this comment.
I tried to add it back and got:
Configuration issues (1)
Unused item in ignoreDependencies: @khanacademy/wonder-blocks-tokens
✂️ Excellent, Knip found no issues.
Kind of mixed signals...
2 participants
Summary:
Inspired by this Slack convo
We don't need jQuery as much today as we did back in the day. I asked the LLM to remove some low-risk uses of jQuery and this is what it did.