Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix search in code editor #884

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix search in code editor #884

wants to merge 3 commits into from

Conversation

cben
Copy link

@cben cben commented Jun 5, 2023

Search within code was broken on https://elm-lang.org/try and all examples 😿

  1. CodeMirror's builtin search bar was invisible => copied missing dialog.css from
    https://github.com/codemirror/codemirror5/blob/5.40.2/addon/dialog/dialog.css

    before after
    bildo bildo
  2. Switched CodeMirror's search to "findPersistent" which behaves closer to browser's search.

  3. Sometimes user will encounter browser's search anyway (if focus was outside editor, via menu, in Chrome on 2nd Ctrl-F press...).
    Tuned CodeMirror to keep up to 500 off-screen lines in DOM (default was 10), allowing browser's search to Just Work accurately on all elm examples.
    (not setting Infinity to avoid slow editor in case user pastes very long code. CM's builtin search is always accurate.)

cben added 3 commits June 5, 2023 09:53
Search within the code editor was broken in all examples.

CodeMirror "search" & related addons were included,
so CodeMirror handles Ctrl-F, Ctrl-G and related keys
(compare demo https://codemirror.net/5/demo/search.html)
and it prompts for the string using the "dialog" addon.
Alas, the `.CodeMirror-dialog` div was unstyled, positioned off-screen.

Copied `addon/dialog/dialog.css` from CM 5.40.2 which has the necessary
`position: absolute` etc. to render it over the top of the editor.

- Also fixed `addon/dialog/dialog.js` being bundled twice.
…rowser

Browsers' native search (at least Chrome & Firefox) nowdays does
incremental search as you type, and lets you jump to next/prev matches
by repeatedly pressing Enter/Shift-Enter.

CodeMirror's default "find" action closes the search bar on first Enter.
It does highlight all matches, but jumping to next/prev matches requires
knowing Ctrl-G/Ctrl-Shift-G	keys (less discoverable).
What's worse, if you press Enter again from muscle memory, by that point
the first match is selected, and it gets replaced with a newline.

Switched to "findPersistent" which behaves very similar to browsers
(except it's not incremental).

This similarity is extra important because user is likely to encounter
both – browser search opens if keyboard focus was in the app pane,
or (in Chrome) on pressing Ctrl-F/Ctrl-G a 2nd time in a row.q
CodeMirror's search is more powerful than browser's (has regexps) but
sometimes user will encounter browser's search anyway.

Alas, CodeMirror by default doesn't put all lines in the DOM.
E.g. open "quotes" example, click outside the editor, press Ctrl-F,
search for "field" from top => shows "1/1" match — the import that is in
the viewport — missing 4 more matches at the end.

The longest current elm example is 331 lines.
So setting `viewportMargin: 500` makes browser search work for them all.

I don't want to set `Infinity` — it's possible for user to paste much
longer code, and at some point it'll degrade editor performance, not worth
it for a corner case.  (CodeMirror's own search always finds all matches.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant