processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.41k stars 1.35k forks source link

Ctrl+F glitchy when switching between files #2125

Open greenStone83 opened 1 year ago

greenStone83 commented 1 year ago
  1. Go to this link and go to sketch.js
  2. Ctrl+F and type "test". Notice there are 4 instances of "test"
  3. Switch to new.js and press the "next" (down arrow) on the Ctrl+F bar
  4. Notice it always says 0/4 (it should say 1/2 and 2/2 I assume)
  5. Type another "test" in new.js and hit the down arrow
  6. Notice it says there are now 7 instances of "test" (it should say 3 I assume)

https://editor.p5js.org/greenStone83/sketches/IQMmFIkYv

lindapaiste commented 1 year ago

This a great bug report!

I found where in the code the "0/4" gets set: https://github.com/processing/p5.js-web-editor/blob/9073487232b5f53756c3525981e992cee0fcd0fc/client/utils/codemirror-search.js#L456-L459

I'm kind of surprised that none of it is React. The text of the DOM element is manually changed when the findNext function is called. I would have to dig into this more to fix it, as I'm not familiar with CodeMirror. I suspect that some of these issues of outdated state could be resolved by generating the search bar markup in React, even if we still have to use react-dom/renderToString() to pass it the CodeMirror openDialog. But I'm not sure that it's even necessary to handle the dialog through CodeMirror, vs. passing the CodeMirror instance as a prop to a React component.

Possibly related to #2081