desmosinc / mathquill

Magic textboxes where you can type math as easily as writing!
mathquill.com
137 stars 42 forks source link

A few broken tests from main versus before-remove-jquery #236

Closed jackschmidt closed 2 years ago

jackschmidt commented 2 years ago

I noticed that a number of the tests in test/visual.html don't work on the main branch, but do on before-remove-jquery

Tested on windows 10 and linux in Firefox and Chrome.

I managed to fix #235 the textcolor one (quoting issue when translating jquery to the new style), but I wasn't sure what was wrong in the other cases.

Clicking in \MathQuillMathField doesn't work

In test/visual.html the x^2+y^2 inside the square root cannot be clicked when using the main branch, but does work in before-remove-jquery.

In both branches you can use tab to get into the square root and edit, but mouse clicks only work on before-remove-jquery.

"Touch taps/clicks/mousedown to drag should work anywhere in the blue box:" but don't

Only clicking directly inside the MathField work. The larger outer box doesn't work for clicking.

Textcolor section doesn't work

"Colors should match their names: " but they are all black

"Nested \textcolor: the 2 should be red, the "a+" green, the 4 blue, and the "+b" green again. " but all are black

Selection test selects a different element

In before-remove-jquery the field in "Selection Tests" is selected, but in main it is the first field on the page.

jwmerrill commented 2 years ago

Wow, thank you for finding these.

jwmerrill commented 2 years ago

Clicking in \MathQuillMathField doesn't work

Bisected to 71df65d3829af6b05634507fa113600619bc52ec

UPDATE: fixed by https://github.com/desmosinc/mathquill/pull/239

jwmerrill commented 2 years ago

Selection test selects a different element

Bisected to 51a75093dac047399e64a0d06a68c937c0df5a3e

UPDATE: fixed by https://github.com/desmosinc/mathquill/pull/237

jwmerrill commented 2 years ago

drag should work anywhere in the blue box

Bisected to 71df65d3829af6b05634507fa113600619bc52ec

UPDATE: fixed by https://github.com/desmosinc/mathquill/pull/240

jackschmidt commented 2 years ago

Thanks for finding the exact commits! I also looked at "drag should work anywhere in the blue box", but I think this part is still using jquery, so I think this is more of a "not yet migrated" rather than a bug.

jwmerrill commented 2 years ago

I also looked at "drag should work anywhere in the blue box", but I think this part is still using jquery, so I think this is more of a "not yet migrated" rather than a bug.

My guess is that the part that's failing is the triggerHandler in https://github.com/desmosinc/mathquill/blob/60d51cc4c6f08b0d3be4b871fd49b91c55b119d0/test/visual.html#L1057

MQ won't be listening for jQuery triggered events anymore, so I think we'll need to adjust this to listen to/forward native DOM events instead.

jwmerrill commented 2 years ago

@jackschmidt just wanted to say thanks again for this very detailed report and the fixes.

Desmos is currently working on getting our work merged upstream (https://github.com/mathquill/mathquill/pull/947 and a few related PRs), and a few of us are hoping to rebuild some momentum on the upstream open source project. If there are more contributions you'd like to make, I can try to help you coordinate.