summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFatih Acet <acetfatih@gmail.com>2016-10-25 18:34:52 +0000
committerFatih Acet <acetfatih@gmail.com>2016-10-25 18:34:52 +0000
commit74c0e758668d3daf2640fbd2198c5dd5ed2e0893 (patch)
tree4b6a11d761dc614e77557c8e70dfab2b27d76d8b
parent067bbd71973409be1d8f6aaa8ef3bfc7c862b2e4 (diff)
parentf1bb7ddfa76e73fc67922637bbfaf17d83df7ddc (diff)
downloadgitlab-ce-74c0e758668d3daf2640fbd2198c5dd5ed2e0893.tar.gz
Merge branch 'itchy-trigger-finger-on-issues-search' into 'master'
Make issues search less finicky ## What does this MR do? 1. Tracks issues search pristine-ness, to ignore non-mutating keyups. 2. Increase debounce wait on issues search execution from 500ms to 1000ms. 3. Ensures issues search retains focus after search execution Note: Issues search is being overhauled (https://gitlab.com/gitlab-org/gitlab-ce/issues/21747), so most (if not all) of these changes will no longer be used. But given that the overhaul has been pushed back a release (8.14?), it makes sense to do some quick fixes to improve UX in the meantime. ## Are there points in the code the reviewer needs to double check? Will adding autofocus to the search input create unforeseen problems? ## Why was this MR needed? - At the moment, issues search is run on any keyup, even if search terms remain the same. This is an oversight that is both a tax on servers and an annoyance to users. - Searches are executed pretty quickly after a gap in keyups. It's too fast according to internal and enterprise customer feedback. - Focus is lost when a search is conducted, so you have to either tab to (any sane person would not do this, given our tab order) or reach for the mouse and select the input again. These are all pretty heavily complained about issues that are, to quote community users, "rage-inducing" and "major accessibility issue[s]". ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [x] API support added - Tests - [x] All builds are passing - [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? https://gitlab.com/gitlab-org/gitlab-ce/issues/21503 https://gitlab.com/gitlab-org/gitlab-ce/issues/21984 https://gitlab.com/gitlab-org/gitlab-ce/issues/21597 See merge request !6735
-rw-r--r--CHANGELOG.md1
-rw-r--r--app/assets/javascripts/issuable.js.es656
2 files changed, 54 insertions, 3 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5ec5fd9a898..0989345d230 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -105,6 +105,7 @@ Please view this file on the master branch, on stable branches it's out of date.
- Add RTL support to markdown renderer (Ebrahim Byagowi)
- Add word-wrap to issue title on issue and milestone boards (ClemMakesApps)
- Fix todos page mobile viewport layout (ClemMakesApps)
+ - Make issues search less finicky
- Fix inconsistent highlighting of already selected activity nav-links (ClemMakesApps)
- Remove redundant mixins (ClemMakesApps)
- Added 'Download' button to the Snippets page (Justin DiPierro)
diff --git a/app/assets/javascripts/issuable.js.es6 b/app/assets/javascripts/issuable.js.es6
index 57f7e4ef230..54056ac50c8 100644
--- a/app/assets/javascripts/issuable.js.es6
+++ b/app/assets/javascripts/issuable.js.es6
@@ -15,16 +15,61 @@
return Issuable.labelRow = _.template('<% _.each(labels, function(label){ %> <span class="label-row btn-group" role="group" aria-label="<%- label.title %>" style="color: <%- label.text_color %>;"> <a href="#" class="btn btn-transparent has-tooltip" style="background-color: <%- label.color %>;" title="<%- label.description %>" data-container="body"> <%- label.title %> </a> <button type="button" class="btn btn-transparent label-remove js-label-filter-remove" style="background-color: <%- label.color %>;" data-label="<%- label.title %>"> <i class="fa fa-times"></i> </button> </span> <% }); %>');
},
initSearch: function() {
+ const $searchInput = $('#issuable_search');
+
+ Issuable.initSearchState($searchInput);
+
// `immediate` param set to false debounces on the `trailing` edge, lets user finish typing
- const debouncedExecSearch = _.debounce(Issuable.executeSearch, 500, false);
+ const debouncedExecSearch = _.debounce(Issuable.executeSearch, 1000, false);
- $('#issuable_search').off('keyup').on('keyup', debouncedExecSearch);
+ $searchInput.off('keyup').on('keyup', debouncedExecSearch);
// ensures existing filters are preserved when manually submitted
- $('#issue_search_form').on('submit', (e) => {
+ $('#issuable_search_form').on('submit', (e) => {
e.preventDefault();
debouncedExecSearch(e);
});
+
+ },
+ initSearchState: function($searchInput) {
+ const currentSearchVal = $searchInput.val();
+
+ Issuable.searchState = {
+ elem: $searchInput,
+ current: currentSearchVal
+ };
+
+ Issuable.maybeFocusOnSearch();
+ },
+ accessSearchPristine: function(set) {
+ // store reference to previous value to prevent search on non-mutating keyup
+ const state = Issuable.searchState;
+ const currentSearchVal = state.elem.val();
+
+ if (set) {
+ state.current = currentSearchVal;
+ } else {
+ return state.current === currentSearchVal;
+ }
+ },
+ maybeFocusOnSearch: function() {
+ const currentSearchVal = Issuable.searchState.current;
+ if (currentSearchVal && currentSearchVal !== '') {
+ const queryLength = currentSearchVal.length;
+ const $searchInput = Issuable.searchState.elem;
+
+ /* The following ensures that the cursor is initially placed at
+ * the end of search input when focus is applied. It accounts
+ * for differences in browser implementations of `setSelectionRange`
+ * and cursor placement for elements in focus.
+ */
+ $searchInput.focus();
+ if ($searchInput.setSelectionRange) {
+ $searchInput.setSelectionRange(queryLength, queryLength);
+ } else {
+ $searchInput.val(currentSearchVal);
+ }
+ }
},
executeSearch: function(e) {
const $search = $('#issuable_search');
@@ -32,6 +77,11 @@
const $searchValue = $search.val();
const $filtersForm = $('.js-filter-form');
const $input = $(`input[name='${$searchName}']`, $filtersForm);
+ const isPristine = Issuable.accessSearchPristine();
+
+ if (isPristine) {
+ return;
+ }
if (!$input.length) {
$filtersForm.append(`<input type='hidden' name='${$searchName}' value='${_.escape($searchValue)}'/>`);