Feat(math): Improve legacy math input with MathLive #7842
Conversation
dosubot
bot
added
the
size:XL
Summary of ChangesHello @Meinzzzz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the math input functionality by integrating the MathLive library, offering users a more intuitive and powerful way to create and edit mathematical equations. The changes introduce a dual-interface approach, allowing users to switch between a visual equation editor and a direct LaTeX code editor, ensuring a flexible and robust math-editing experience within the application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a significant enhancement, integrating MathLive to provide a much richer math editing experience. The implementation is comprehensive, covering the new component views, two-way data synchronization between the visual and raw LaTeX editors, resize handling, and updated tests. The code is well-structured, and the attention to details like cleaning up resources in the destroy method is commendable. I've identified a few areas for improvement, including a bug in the CSS, an enhancement to the resize synchronization logic, and a couple of refactoring opportunities to improve code clarity and robustness. Overall, this is a high-quality contribution.
eliandoran
left a comment
eliandoran
left a comment
There was a problem hiding this comment.
I've tried testing it but it fails with:
Uncaught (in promise) TypeError: can't access property "bind", formView.mathLiveInputView is undefined
_createFormView mathui.ts:92
init mathui.ts:29
promise callback*init/initPlugins/< chunk-CZONVLGZ.js:45733
initPlugins chunk-CZONVLGZ.js:45726
init chunk-CZONVLGZ.js:45614
initPlugins chunk-CZONVLGZ.js:47131
create ckeditor5.js:9558
create ckeditor5.js:9553
buildEditor CKEditorWithWatchdog.tsx:222
CKEditorWithWatchdog CKEditorWithWatchdog.tsx:158
| this._resizeObserver = new ResizeObserver( entries => { | ||
| if ( !this._activeResizeTarget ) { | ||
| return; | ||
| } | ||
|
|
||
| for ( const entry of entries ) { | ||
| if ( entry.target === this._activeResizeTarget ) { | ||
| // Use style.width directly to avoid box-sizing issues causing infinite growth | ||
| const width = ( entry.target as HTMLElement ).style.width; | ||
|
|
||
| if ( !width ) continue; | ||
|
|
||
| const other = entry.target === this.mathLiveInputView.element | ||
| ? this.rawLatexInputView.element | ||
| : this.mathLiveInputView.element; | ||
|
|
||
| if ( other && other.style.width !== width ) { | ||
| window.requestAnimationFrame( () => { | ||
| other.style.width = width; | ||
| } ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This whole resize mechanism feels overengineered. Do we actually need it? I tried remove part of it and it worked just fine.
I would prefer not to have this logic, maybe see if we can work around it using CSS since the popup follows the biggest text area, I think it should be fine without it.
There was a problem hiding this comment.
I think I found a good solution that works using only CSS and also feels nice to use. Instead of making both boxes resizable, only the LaTeX box remains resizable. The Mathlive component can then grow automatically depending on the input.
|
Functionally speaking, it's pretty interesting since it makes it relatively easy for people not familiar with LaTeX to insert equations.
|
|
@Meinzzzz , I see you've marked the PR as ready but no changes were pushed. Perhaps you forgot to push your changes? |
|
Sorry, i hit the revie button by accident. |
Let's keep the size constraints for later after we run a nightly build on it to determine the impact of the changes. |
2 participants
Description :
Integrates a MathLive field to provide a richer math-editing experience, offering both a visual equation editor and a raw LaTeX code editor.
Key Features
Visual equation editing using MathLive, while preserving an area for directly editing raw LaTeX.
Technical Implementation:
Testing:Unit Tests: Added comprehensive test suites for MathUI, covering:
Dependencies: