Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
Find/Replace UI:
Browse files Browse the repository at this point in the history 
* Treat Enter as Find Next instead of closing the bar. Removes Enter key
handling and "commit" event from ModalBar.
* Move error message into popup & clean up how its shown/hidden. Make text
field border red whenever error is shown. Same for FindInFiles, as well.
* Remove redundant "Skip" button in Replace UI
* Make next/prev and Replace/All button pairs flush
* Move result-count label inside search field
* Fix all existing unit tests (removing some that are no longer applicable
with the new Enter key behavior).
  • Loading branch information
@peterflynn
peterflynn committed Dec 4, 2013
1 parent 3de8421 commit 5cd2043
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 365 deletions.
3 changes: 1 addition & 2 deletions src/htmlContent/search-dialog.html
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{{CMD_FIND}}:
<input type="text" id="searchInput" value="{{value}}" style="width: 10em" />
<div class="search-input-container"><input type="text" id="searchInput" value="{{value}}" /><div class="error"></div></div>
<div class="message">
<span id="searchlabel">{{{label}}}</span>
<span style="color: #888">({{SEARCH_REGEXP_INFO}})</span>
</div>
<div class="error"></div>
1 change: 0 additions & 1 deletion src/nls/root/strings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ define({
"BUTTON_YES" : "Yes",
"BUTTON_NO" : "No",
"BUTTON_REPLACE_ALL" : "All\u2026",
"BUTTON_SKIP" : "Skip",
"BUTTON_REPLACE" : "Replace",

"BUTTON_NEXT" : "\u25B6",
Expand Down
21 changes: 9 additions & 12 deletions src/search/FindInFiles.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ define(function (require, exports, module) {
}

// Clear any pending RegEx error message
$(".modal-bar .message").css("display", "inline-block");
$(".modal-bar .error").css("display", "none");
$(".modal-bar .error").hide();

// If query is a regular expression, use it directly
var isRE = query.match(/^\/(.*)\/(g|i)*$/);
Expand All @@ -138,17 +137,14 @@ define(function (require, exports, module) {
try {
return new RegExp(isRE[1], flags);
} catch (e) {
$(".modal-bar .message").css("display", "none");
$(".modal-bar .error")
.css("display", "inline-block")
.html("<div class='alert' style='margin-bottom: 0'>" + e.message + "</div>");
.show()
.text(e.message);
return null;
}
}

// Query is a string. Turn it into a case-insensitive regexp

// Escape regex special chars
// Query is a plain string. Turn it into a case-insensitive regexp
query = StringUtils.regexEscape(query);
return new RegExp(query, "gi");
}
Expand Down Expand Up @@ -488,8 +484,7 @@ define(function (require, exports, module) {
.removeAttr("disabled")
.get(0).select();

$(".modal-bar .message").css("display", "none");
$(".modal-bar .error").css("display", "inline-block").html(Strings.FIND_NO_RESULTS);
$(".modal-bar .error").show().text(Strings.FIND_NO_RESULTS);
}
}
}
Expand Down Expand Up @@ -758,6 +753,8 @@ define(function (require, exports, module) {
that = this;

this.modalBar = new ModalBar(dialogHTML, false);
$(".modal-bar .error").hide();

var $searchField = $("input#searchInput");

$searchField.get(0).select();
Expand All @@ -781,8 +778,8 @@ define(function (require, exports, module) {
.bind("input", function (event) {
// Check the query expression on every input event. This way the user is alerted
// to any RegEx syntax errors immediately.
_getQueryRegExp($searchField.val());
that.getDialogTextField().removeClass("no-results");
var query = _getQueryRegExp($searchField.val());
that.getDialogTextField().toggleClass("no-results", (query === null));
})
.blur(function () {
if (that.getDialogTextField().attr("disabled")) {
Expand Down
87 changes: 47 additions & 40 deletions src/search/FindReplace.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ define(function (require, exports, module) {
Editor = require("editor/Editor"),
EditorManager = require("editor/EditorManager"),
ModalBar = require("widgets/ModalBar").ModalBar,
KeyEvent = require("utils/KeyEvent"),
ScrollTrackMarkers = require("search/ScrollTrackMarkers"),
PanelManager = require("view/PanelManager"),
Resizer = require("utils/Resizer"),
Expand Down Expand Up @@ -97,19 +98,19 @@ define(function (require, exports, module) {

function parseQuery(query) {
var isRE = query.match(/^\/(.*)\/([a-z]*)$/);
$(".modal-bar .message").css("display", "inline-block");
$(".modal-bar .error").css("display", "none");
$(".modal-bar .message").show();
$(".modal-bar .error").hide();
try {
if (isRE && isRE[1]) { // non-empty regexp
return new RegExp(isRE[1], isRE[2].indexOf("i") === -1 ? "" : "i");
} else {
return query;
}
} catch (e) {
$(".modal-bar .message").css("display", "none");
$(".modal-bar .message").hide();
$(".modal-bar .error")
.css("display", "inline-block")
.html("<div class='alert' style='margin-bottom: 0'>" + e.message + "</div>");
.show()
.text(e.message);
return "";
}
}
Expand Down Expand Up @@ -186,39 +187,35 @@ define(function (require, exports, module) {
});
}

function createModalBar(template, autoClose) {
function createModalBar(template) {
// Normally, creating a new modal bar will simply cause the old one to close
// automatically. This can cause timing issues because the focus change might
// cause the new one to think it should close, too. The old CodeMirror version
// of this handled it by adding a timeout within which a blur wouldn't cause
// the modal bar to close. Rather than reinstate that hack, we simply explicitly
// close the old modal bar before creating a new one.
if (modalBar) {
// 1st arg = restore scroll pos; 2nd arg = no animation, since getting replaced immediately
modalBar.close(true, false);
}
modalBar = new ModalBar(template, autoClose);
modalBar = new ModalBar(template, true); // 2nd arg = auto-close on Esc/blur

$(modalBar).on("commit close", function (event) {
$(modalBar).on("close", function (event) {
modalBar = null;
});
}

var findDialog =
"<input type='text' id='find-what'/>" +
"<div class='search-input-container'><input type='text' id='find-what'/><div class='error'></div><span id='find-counter'></span></div>" +
"<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>" +
"<div class='message'>" +
"<span id='find-counter'></span> " +
"</div>" +
"<div class='error'></div>";
"</div>";

var replaceDialog = findDialog +
"<input type='text' id='replace-with' placeholder='" + Strings.REPLACE_PLACEHOLDER + "'/>" +
"<button id='replace-yes' class='btn'>" + Strings.BUTTON_REPLACE +
"</button> <button id='replace-no' class='btn'>" + Strings.BUTTON_SKIP +
"</button> <button id='replace-all' class='btn'>" + Strings.BUTTON_REPLACE_ALL;
"<button id='replace-yes' class='btn'>" + Strings.BUTTON_REPLACE + "</button>" +
"<button id='replace-all' class='btn'>" + Strings.BUTTON_REPLACE_ALL + "</button>";


function toggleHighlighting(editor, enabled) {
Expand All @@ -241,12 +238,12 @@ define(function (require, exports, module) {
state = getSearchState(cm);

function indicateHasMatches(numResults) {
// Make the field red if it's not blank and it has no matches
ViewUtils.toggleClass($("#find-what"), "no-results", !state.foundAny && state.query);
// Make the field red if it's not blank and it has no matches (which also covers invalid regexes)
ViewUtils.toggleClass($("#find-what"), "no-results", !state.foundAny && $("#find-what").val());

// Buttons disabled if blank OR no matches (Replace btns) / < 2 matches (nav buttons)
// Buttons disabled if blank, OR if no matches (Replace buttons) / < 2 matches (nav buttons)
$("#find-prev, #find-next").prop("disabled", !state.foundAny || numResults < 2);
$("#replace-yes, #replace-no, #replace-all").prop("disabled", !state.foundAny);
$("#replace-yes, #replace-all").prop("disabled", !state.foundAny);
}

cm.operation(function () {
Expand Down Expand Up @@ -356,23 +353,36 @@ define(function (require, exports, module) {
}

// Create the search bar UI (closing any previous modalBar in the process)
createModalBar(template, true);
createModalBar(template);

$(modalBar).on("commit close", function (e, query) {
$(modalBar).on("close", function (e, query) {
// Clear highlights but leave search state in place so Find Next/Previous work after closing
clearHighlights(cm, state);

// Dispose highlighting UI (important to restore normal selection color as soon as focus goes back to the editor)
toggleHighlighting(editor, false);

// Hide error popup, since it hangs down low enough to make the slide-out look awkward
$(".modal-bar .error").hide();
});

modalBar.getRoot().on("click", function (e) {
if (e.target.id === "find-next") {
findNext(editor);
} else if (e.target.id === "find-prev") {
findNext(editor, true);
}
});
modalBar.getRoot()
.on("click", function (e) {
if (e.target.id === "find-next") {
findNext(editor);
} else if (e.target.id === "find-prev") {
findNext(editor, true);
}
})
.on("keydown", function (e) {
if (e.keyCode === KeyEvent.DOM_VK_RETURN) {
if (!e.shiftKey) {
findNext(editor);
} else {
findNext(editor, true);
}
}
});

$("#find-what").on("input", function () {
handleQueryChange(editor, state);
Expand Down Expand Up @@ -543,21 +553,18 @@ define(function (require, exports, module) {
return $("#replace-with").val() || "";
}

function advance() {
if (!findNext(editor)) {
// No more matches, so destroy modalBar
modalBar.close();
}
}

modalBar.getRoot().on("click", function (e) {
if (e.target.id === "replace-yes") {
var text = getReplaceWith();
cm.replaceSelection(typeof state.query === "string" ? text : parseDollars(text, state.lastMatch));

updateResultSet(editor); // we updated the text, so result count & tickmarks must be refreshed
advance();
} else if (e.target.id === "replace-no") {
advance();

if (!findNext(editor)) {
// No more matches, so destroy modalBar
modalBar.close();
}

} else if (e.target.id === "replace-all") {
modalBar.close();
_showReplaceAllPanel(editor, state.query, getReplaceWith());
Expand Down
56 changes: 44 additions & 12 deletions src/styles/brackets.less
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1006,7 +1006,7 @@ a, img {
font-size: 14px;
color: @tc-text;
background: @tc-gray-panel;
overflow: hidden;
overflow: visible; // needed for .error popup
padding: 5px 4px 4px 14px;

-webkit-transform: translate(0, 0); // Prefix still required.
Expand Down Expand Up @@ -1050,21 +1050,41 @@ a, img {
}
}

#find-what, #replace-with {
width: 20em;
#replace-with, #searchInput {
width: 295px;
}

.message {
display: inline-block;
#find-what {
padding-right: 65px; // room for #find-counter overlay
width: 295px - (65px - 6px); // keep same width as other fields, after accounting for differing padding
}

.error {
display: none;

.alert {
padding-top: 4px;
padding-bottom: 4px;
.search-input-container {
position: relative;
display: inline;

.error { // "popup" that hangs below search field
position: absolute;
left: 6px;
top: 24px;
min-width: 295px + 2px; // to align with search field above it

background-color: #f74687;
color: @bc-white;
font-size: 12px;
padding: 5px;
border-radius: 0 0 3px 3px;
}
#find-counter {
position: absolute;
color: @tc-light-weight-quiet-text;
top: 1px;
right: 5px;
font-size: 12px;
}
}

.message {
display: inline-block;
}

.navigator {
Expand All @@ -1073,6 +1093,18 @@ a, img {
margin: 2px 1px 3px;
}
}

// Make button pairs snug
#find-prev, #replace-yes {
border-top-right-radius: 0;
border-bottom-right-radius: 0;
margin-right: 0;
}
#find-next, #replace-all {
border-top-left-radius: 0;
border-bottom-left-radius: 0;
margin-left: -1px;
}
}

// Search result highlighting - CodeMirror highlighting is pretty constrained. Highlights are
Expand Down
Loading

0 comments on commit 5cd2043

Please sign in to comment.