Improved Find/Replace UI

[peterflynn]

Not done yet, but pushed in case anyone else (@larz0?) wants to play with it.

Finished so far:

• Replace is now a single UI rather than a 3-stage "wizard." Instead of Yes/No/Stop, there's a single Replace button.
• Replace now shares most of its code with Find, so Replace gets incremental search, match highlighting, match counting, and tickmark indicators.
• Enter is treated as Find Next instead of closing the search bar. (And Shift+Enter = Find Prev)
• Streamlined UI: match count hovers inside query field, regexp errors appear in a popup below the query field, next/prev buttons don't dynamically show/hide, related buttons are clustered tightly, and eliminated labels in favor of prompt text inside the replace field.
• Explicit toggles for regexp & case-insensitive -- no longer auto-detecting based on what the query string looks like. Toggles are shared across Find, Replace, & FindInFiles and are persisted as a pref.
• Find Next uses editor cursor pos rather than an invisible saved pos (this is part of the future Correct Find Next Behavior story, but it simplified the code here so I went ahead & did it now).
• Keyboard shortcut for Replace button -- just use the regular Replace shortcut while the bar is open. So you can do a replace operation entirely from the keyboard, just using the Find Next & Replace shortcuts.
• Unit tests

Not implemented yet:

• Decide whether to unify Find & Replace into a single command (i.e. you'd never see the simpler Find-only UI anymore)
• Final visual polish -- make search/replace fields dynamically fill available width? Etc.

Things to test:

• Find
• Replace
• Replace All (no functionality changed inside the panel, but make sure launching the panel works right)
• Find in Files

[TomMalbran]

It might be a good time to join this template with the Find in Files ones and use Mustache to decide what to display or not, since both have several similar parts and will have more once we have more Find in Files features. At the moment it could be done like:

<div class="search-input-container">
    <input type="text" id="find-what" value="{{value}}" placeholder="{{Strings.CMD_FIND}}" />
    <div class="error"></div>
    <span id="find-counter"></span>
</div>
<button id="find-case-sensitive" class="btn no-focus" tabindex="-1" title="{{Strings.BUTTON_CASESENSITIVE_HINT}}"><div class="button-icon"></div></button>
<button id="find-regexp" class="btn no-focus" tabindex="-1" title="{{Strings.BUTTON_REGEXP_HINT}}"><div class="button-icon"></div></button>

{{#hasPrevNext}}
<div class="navigator">
    <button id="find-prev" class="btn no-focus" tabindex="-1" title="{{Strings.BUTTON_PREV_HINT}}">{{Strings.BUTTON_PREV}}</button>
    <button id="find-next" class="btn no-focus" tabindex="-1" title="{{Strings.BUTTON_NEXT_HINT}}">{{Strings.BUTTON_NEXT}}</button>
</div>
{{/hasPrevNext}}

{{#hasReplace}}
<input type="text" id="replace-with" placeholder="{{Strings.REPLACE_PLACEHOLDER}}"/>
<button id="replace-yes" class="btn no-focus" tabindex="-1">{{Strings.BUTTON_REPLACE}}</button>
<button id="replace-all" class="btn no-focus" tabindex="-1">{{Strings.BUTTON_REPLACE_ALL}}</button>
{{/hasReplace}}

<div class="no-results-message">{{FIND_NO_RESULTS}}</div></div>
{{#label}}
<div class="message">
    <span id="searchlabel">{{{label}}}</span>
</div>
{{/label}}

And then Create the template with:

Mustache.render(searchDialogTemplate, {
    value       : initialQuery || "",
    label       : "",     // Will have a value in Find in Files
    hasPrevNext : true,   // False for Find in Files
    hasReplace  : false,  // True for Replace
    Strings     : Strings
});

[couzteau]

2nd that. Much prefer to have HTML code in a template file

[peterflynn]

I've combined the Find & Replace HTML fully, and I'll move it into an external file, but I'd like to keep this separate from the Find in Files UI for now because I expect they will diverge further over time (e.g. Find in Files will have additional options for choosing the set of files, etc.) and it's not clear yet how much would be shared.

[peterflynn]

Unfortunately this template is pretty ugly because horizontal (inline/inline-block layouts are extremely sensitive to whitespace). So it either has to be minified, or have every line start/end with a comment...

[TomMalbran]

Sure. Makes sense to keep them separate. Maybe we could use partials to reuse the parts that dont change between both templates. Like the input and buttons.

[couzteau]

Q: Is it's necessary to update the Copyright notes for FindReplace, FindInFiles ... to 2013?

[peterflynn]

@couzteau For existing files we don't need to increment the copyright year. Only newly-created files need the latest year.

[couzteau]

Will FindReplace and FindInFiles be merged later? wondering wether a e method such as updatePrefsFromSearchBar should live in a module such as FindUtils, I suppose there are more candidates for such refactoring, ie handleQueryChange

[peterflynn]

See my note above. I don't expect they'll ever be fully combined because there are just too many differences. But they could certainly share more code in the future than they do today. I think that's probably out of scope for this story, though.

[TomMalbran]

It will still be nice to move this new functions to a new utils file and refactor more code on later PRs.

[peterflynn]

There's so much churn coming in this code with the overall Find epic, that I'd like to to try to clean it up too heavily right away. Let's let the dust start settling a tiny bit more and then see what the common patterns are.

[TomMalbran]

Sure. I was thinking that we could later try to join the Find Modal Bars objects in one, with most of the regexp stuff there and then just extend them in Find in Files or Find when is required. I think that this is where most of the stuff is shared at the moment. New things might come up with the next features.

[couzteau]

I love the side effect of being able to use find replace as interactive regex tester!

[couzteau]

Has there been an API change in CodeMirror? Wondering because the new implementation does not return anything to cm.operation anymore

[peterflynn]

On master this line is the same: we don't return a value to cm.operation(). Afaik operation() has never expected or used any return values.

You might be thinking of the return found; line that's removed below -- which is outside the operation() call, returning from findNext() overall. I removed that as a cleanup because this is now recorded in state.lastMatch.

[couzteau]

got it. The diff is difficult to read.

[couzteau]

Done with initial review.

[peterflynn]

@couzteau Changes pushed. The only thing I haven't addressed yet is Raymond's comment about switching directly between the Find/Replace & Find in Files bars with the keyboard. I have a fix for that but since it's a little hacky I'd like to test it locally a bit more first before pushing it up.

[RaymondLim]

Instead of having to pad with an empty html comment, you can achieve the same layout by just moving down > from the prior end tag as in the following example.

<div class="search-input-container"><input type="text" id="find-what" value="{{value}}" /><div class="error"></div
  ><button id='find-case-sensitive' class='btn no-focus' tabindex='-1' title='{{BUTTON_CASESENSITIVE_HINT}}'><div class='button-icon'></div></button>

[peterflynn]

That might be even harder to read than the comment hack though... anyone else have an opinion or idea for a good way to get around this?

[redmunds]

Idea is to remove the gaps between inline elements, correct? Have you tried using a flex box layout in the dialog? I think it can also be done if you put float:left or display:inline-block on everything.

[TomMalbran]

Felxbox will work great if we want the input fields take the whole left over space split in an equal way. The same way we used it for the Find in Files and Replace tiles in the bottom panel.

[peterflynn]

@couzteau Changes pushed to address @RaymondLim's double-quotes comment and the bug he found on Fri related to switching between Find in Files and Find/Replace.

[couzteau]

Looks good now. Merging.