Skip to content

G
gitlab-ce
Commit 1356e40f authored by Luke Bennett's avatar Luke Bennett Committed by Luke "Jared" Bennett
Browse files
Options

Changed autocomplete_sources into an action that returns a single 'at' type of sources at a time 

Finished up autocomplete_sources action and added frontend to fetch data only when its needed

Added wait_for_ajax to specs

Fixed builds and improved the setup/destroy lifecycle

Changed global namespace and DRYed up loading logic

Added safety for accidentally loading data twice

Removed destroy as its not necessary and is messing with click events from a blur race condition

Created AutocompleteSourcesController and updated routes

Fixed @undefined from tabbing before load ends

Disable tabSelectsMatch until we have loaded data

Review changes
parent 677e7e83
Show whitespace changes
Inline Side-by-side
Showing with 210 additions and 169 deletions
app/assets/javascripts/gfm_auto_complete.js.es6
View file @ 1356e40f
......@@ -2,19 +2,28 @@
// Creates the variables for setting up GFM auto-completion
(function() {
if (window.GitLab == null) {
window.GitLab = {};
if (window.gl == null) {
window.gl = {};
}
function sanitize(str) {
return str.replace(/<(?:.|\n)*?>/gm, '');
}
window.GitLab.GfmAutoComplete = {
dataLoading: false,
dataLoaded: false,
window.gl.GfmAutoComplete = {
dataSources: {},
defaultLoadingData: ['loading'],
cachedData: {},
dataSource: '',
isLoadingData: {},
atTypeMap: {
':': 'emojis',
'@': 'members',
'#': 'issues',
'!': 'mergeRequests',
'~': 'labels',
'%': 'milestones',
'/': 'commands'
},
// Emoji
Emoji: {
template: '<li>${name} <img alt="${name}" height="20" src="${path}" width="20" /></li>'
......@@ -35,33 +44,31 @@
template: '<li>${title}</li>'
},
Loading: {
template: '<li><i class="fa fa-refresh fa-spin"></i> Loading...</li>'
template: '<li style="pointer-events: none;"><i class="fa fa-refresh fa-spin"></i> Loading...</li>'
},
DefaultOptions: {
sorter: function(query, items, searchKey) {
// Highlight first item only if at least one char was typed
this.setting.highlightFirst = this.setting.alwaysHighlightFirst || query.length > 0;
if ((items[0].name != null) && items[0].name === 'loading') {
if (gl.GfmAutoComplete.isLoading(items)) {
return items;
}
return $.fn.atwho["default"].callbacks.sorter(query, items, searchKey);
},
filter: function(query, data, searchKey) {
if (data[0] === 'loading') {
if (gl.GfmAutoComplete.isLoading(data)) {
gl.GfmAutoComplete.togglePreventSelection.call(this, true);
gl.GfmAutoComplete.fetchData(this.$inputor, this.at);
return data;
}
} else {
gl.GfmAutoComplete.togglePreventSelection.call(this, false);
return $.fn.atwho["default"].callbacks.filter(query, data, searchKey);
}
},
beforeInsert: function(value) {
if (value && !this.setting.skipSpecialCharacterTest) {
var withoutAt = value.substring(1);
if (withoutAt && /[^\w\d]/.test(withoutAt)) value = value.charAt() + '"' + withoutAt + '"';
}
if (!window.GitLab.GfmAutoComplete.dataLoaded) {
return this.at;
} else {
return value;
}
},
matcher: function (flag, subtext) {
// The below is taken from At.js source
......@@ -85,69 +92,46 @@
}
}
},
setup: _.debounce(function(input) {
setup: function(input) {
// Add GFM auto-completion to all input fields, that accept GFM input.
this.input = input || $('.js-gfm-input');
// destroy previous instances
this.destroyAtWho();
// set up instances
this.setupAtWho();
if (this.dataSource && !this.dataLoading && !this.cachedData) {
this.dataLoading = true;
return this.fetchData(this.dataSource)
.done((data) => {
this.dataLoading = false;
this.loadData(data);
this.setupLifecycle();
},
setupLifecycle() {
this.input.each((i, input) => {
const $input = $(input);
$input.off('focus.setupAtWho').on('focus.setupAtWho', this.setupAtWho.bind(this, $input));
});
};
if (this.cachedData != null) {
return this.loadData(this.cachedData);
}
}, 1000),
setupAtWho: function() {
},
setupAtWho: function($input) {
// Emoji
this.input.atwho({
$input.atwho({
at: ':',
displayTpl: (function(_this) {
return function(value) {
if (value.path != null) {
return _this.Emoji.template;
} else {
return _this.Loading.template;
}
};
})(this),
displayTpl: function(value) {
return value.path != null ? this.Emoji.template : this.Loading.template;
}.bind(this),
insertTpl: ':${name}:',
data: ['loading'],
startWithSpace: false,
skipSpecialCharacterTest: true,
data: this.defaultLoadingData,
callbacks: {
sorter: this.DefaultOptions.sorter,
filter: this.DefaultOptions.filter,
beforeInsert: this.DefaultOptions.beforeInsert,
matcher: this.DefaultOptions.matcher
filter: this.DefaultOptions.filter
}
});
// Team Members
this.input.atwho({
$input.atwho({
at: '@',
displayTpl: (function(_this) {
return function(value) {
if (value.username != null) {
return _this.Members.template;
} else {
return _this.Loading.template;
}
};
})(this),
displayTpl: function(value) {
return value.username != null ? this.Members.template : this.Loading.template;
}.bind(this),
insertTpl: '${atwho-at}${username}',
searchKey: 'search',
data: ['loading'],
startWithSpace: false,
alwaysHighlightFirst: true,
skipSpecialCharacterTest: true,
data: this.defaultLoadingData,
callbacks: {
sorter: this.DefaultOptions.sorter,
filter: this.DefaultOptions.filter,
......@@ -178,20 +162,14 @@
}
}
});
this.input.atwho({
$input.atwho({
at: '#',
alias: 'issues',
searchKey: 'search',
displayTpl: (function(_this) {
return function(value) {
if (value.title != null) {
return _this.Issues.template;
} else {
return _this.Loading.template;
}
};
})(this),
data: ['loading'],
displayTpl: function(value) {
return value.title != null ? this.Issues.template : this.Loading.template;
}.bind(this),
data: this.defaultLoadingData,
insertTpl: '${atwho-at}${id}',
startWithSpace: false,
callbacks: {
......@@ -213,26 +191,21 @@
}
}
});
this.input.atwho({
$input.atwho({
at: '%',
alias: 'milestones',
searchKey: 'search',
displayTpl: (function(_this) {
return function(value) {
if (value.title != null) {
return _this.Milestones.template;
} else {
return _this.Loading.template;
}
};
})(this),
insertTpl: '${atwho-at}${title}',
data: ['loading'],
displayTpl: function(value) {
return value.title != null ? this.Milestones.template : this.Loading.template;
}.bind(this),
startWithSpace: false,
data: this.defaultLoadingData,
callbacks: {
matcher: this.DefaultOptions.matcher,
sorter: this.DefaultOptions.sorter,
beforeInsert: this.DefaultOptions.beforeInsert,
filter: this.DefaultOptions.filter,
beforeSave: function(milestones) {
return $.map(milestones, function(m) {
if (m.title == null) {
......@@ -247,21 +220,15 @@
}
}
});
this.input.atwho({
$input.atwho({
at: '!',
alias: 'mergerequests',
searchKey: 'search',
displayTpl: (function(_this) {
return function(value) {
if (value.title != null) {
return _this.Issues.template;
} else {
return _this.Loading.template;
}
};
})(this),
data: ['loading'],
startWithSpace: false,
displayTpl: function(value) {
return value.title != null ? this.Issues.template : this.Loading.template;
}.bind(this),
data: this.defaultLoadingData,
insertTpl: '${atwho-at}${id}',
callbacks: {
sorter: this.DefaultOptions.sorter,
......@@ -282,18 +249,31 @@
}
}
});
this.input.atwho({
$input.atwho({
at: '~',
alias: 'labels',
searchKey: 'search',
displayTpl: this.Labels.template,
data: this.defaultLoadingData,
displayTpl: function(value) {
return this.isLoading(value) ? this.Loading.template : this.Labels.template;
}.bind(this),
insertTpl: '${atwho-at}${title}',
startWithSpace: false,
callbacks: {
matcher: this.DefaultOptions.matcher,
sorter: this.DefaultOptions.sorter,
beforeInsert: this.DefaultOptions.beforeInsert,
filter: this.DefaultOptions.filter,
beforeSave: function(merges) {
if (gl.GfmAutoComplete.isLoading(merges)) return merges;
var sanitizeLabelTitle;
sanitizeLabelTitle = function(title) {
if (/[\w\?&]+\s+[\w\?&]+/g.test(title)) {
return "\"" + (sanitize(title)) + "\"";
} else {
return sanitize(title);
}
};
return $.map(merges, function(m) {
return {
title: sanitize(m.title),
......@@ -305,12 +285,14 @@
}
});
// We don't instantiate the slash commands autocomplete for note and issue/MR edit forms
this.input.filter('[data-supports-slash-commands="true"]').atwho({
$input.filter('[data-supports-slash-commands="true"]').atwho({
at: '/',
alias: 'commands',
searchKey: 'search',
skipSpecialCharacterTest: true,
data: this.defaultLoadingData,
displayTpl: function(value) {
if (this.isLoading(value)) return this.Loading.template;
var tpl = '<li>/${name}';
if (value.aliases.length > 0) {
tpl += ' <small>(or /<%- aliases.join(", /") %>)</small>';
......@@ -323,7 +305,7 @@
}
tpl += '</li>';
return _.template(tpl)(value);
},
}.bind(this),
insertTpl: function(value) {
var tpl = "/${name} ";
var reference_prefix = null;
......@@ -341,6 +323,7 @@
filter: this.DefaultOptions.filter,
beforeInsert: this.DefaultOptions.beforeInsert,
beforeSave: function(commands) {
if (gl.GfmAutoComplete.isLoading(commands)) return commands;
return $.map(commands, function(c) {
var search = c.name;
if (c.aliases.length > 0) {
......@@ -368,32 +351,40 @@
});
return;
},
destroyAtWho: function() {
return this.input.atwho('destroy');
},
fetchData: function(dataSource) {
return $.getJSON(dataSource);
fetchData: function($input, at) {
if (this.isLoadingData[at]) return;
this.isLoadingData[at] = true;
if (this.cachedData[at]) {
this.loadData($input, at, this.cachedData[at]);
} else {
$.getJSON(this.dataSources[this.atTypeMap[at]], (data) => {
this.loadData($input, at, data);
}).fail(() => { this.isLoadingData[at] = false; });
}
},
loadData: function(data) {
this.cachedData = data;
this.dataLoaded = true;
// load members
this.input.atwho('load', '@', data.members);
// load issues
this.input.atwho('load', 'issues', data.issues);
// load milestones
this.input.atwho('load', 'milestones', data.milestones);
// load merge requests
this.input.atwho('load', 'mergerequests', data.mergerequests);
// load emojis
this.input.atwho('load', ':', data.emojis);
// load labels
this.input.atwho('load', '~', data.labels);
// load commands
this.input.atwho('load', '/', data.commands);
loadData: function($input, at, data) {
this.isLoadingData[at] = false;
this.cachedData[at] = data;
$input.atwho('load', at, data);
// This trigger at.js again
// otherwise we would be stuck with loading until the user types
return $(':focus').trigger('keyup');
return $input.trigger('keyup');
},
isLoading(data) {
if (!data) return false;
if (Array.isArray(data)) data = data[0];
return data === this.defaultLoadingData[0] || data.name === this.defaultLoadingData[0];
},
togglePreventSelection(isPrevented = !!this.setting.tabSelectsMatch) {
this.setting.tabSelectsMatch = !isPrevented;
this.setting.spaceSelectsMatch = !isPrevented;
const eventListenerAction = `${isPrevented ? 'add' : 'remove'}EventListener`;
this.$inputor[0][eventListenerAction]('keydown', gl.GfmAutoComplete.preventSpaceTabEnter);
},
preventSpaceTabEnter(e) {
const key = e.which || e.keyCode;
const preventables = [9, 13, 32];
if (preventables.indexOf(key) > -1) e.preventDefault();
}
};
......
app/assets/javascripts/gl_form.js
View file @ 1356e40f
......@@ -30,7 +30,7 @@
this.form.addClass('gfm-form');
// remove notify commit author checkbox for non-commit notes
gl.utils.disableButtonIfEmptyField(this.form.find('.js-note-text'), this.form.find('.js-comment-button'));
GitLab.GfmAutoComplete.setup(this.form.find('.js-gfm-input'));
gl.GfmAutoComplete.setup(this.form.find('.js-gfm-input'));
new DropzoneInput(this.form);
autosize(this.textarea);
// form and textarea event listeners
......
app/assets/javascripts/issuable_form.js
View file @ 1356e40f
......@@ -19,7 +19,7 @@
this.renderWipExplanation = bind(this.renderWipExplanation, this);
this.resetAutosave = bind(this.resetAutosave, this);
this.handleSubmit = bind(this.handleSubmit, this);
GitLab.GfmAutoComplete.setup();
gl.GfmAutoComplete.setup();
new UsersSelect();
new ZenMode();
this.titleField = this.form.find("input[name*='[title]']");
......
app/controllers/projects/autocomplete_sources_controller.rb 0 → 100644
View file @ 1356e40f
class Projects::AutocompleteSourcesController < Projects::ApplicationController
before_action :load_autocomplete_service, except: [:emojis, :members]
def emojis
render json: Gitlab::AwardEmoji.urls
end
def members
render json: ::Projects::ParticipantsService.new(@project, current_user).execute(noteable)
end
def issues
render json: @autocomplete_service.issues
end
def merge_requests
render json: @autocomplete_service.merge_requests
end
def labels
render json: @autocomplete_service.labels
end
def milestones
render json: @autocomplete_service.milestones
end
def commands
render json: @autocomplete_service.commands(noteable, params[:type])
end
private
def load_autocomplete_service
@autocomplete_service = ::Projects::AutocompleteService.new(@project, current_user)
end
def noteable
case params[:type]
when 'Issue'
IssuesFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:type_id])
when 'MergeRequest'
MergeRequestsFinder.new(current_user, project_id: @project.id).execute.find_by(iid: params[:type_id])
when 'Commit'
@project.commit(params[:type_id])
end
end
end
app/controllers/projects_controller.rb
View file @ 1356e40f
......@@ -127,39 +127,6 @@ class ProjectsController < Projects::ApplicationController
redirect_to edit_project_path(@project), alert: ex.message
end
def autocomplete_sources
noteable =
case params[:type]
when 'Issue'
IssuesFinder.new(current_user, project_id: @project.id).
execute.find_by(iid: params[:type_id])
when 'MergeRequest'
MergeRequestsFinder.new(current_user, project_id: @project.id).
execute.find_by(iid: params[:type_id])
when 'Commit'
@project.commit(params[:type_id])
else
nil
end
autocomplete = ::Projects::AutocompleteService.new(@project, current_user)
participants = ::Projects::ParticipantsService.new(@project, current_user).execute(noteable)
@suggestions = {
emojis: Gitlab::AwardEmoji.urls,
issues: autocomplete.issues,
milestones: autocomplete.milestones,
mergerequests: autocomplete.merge_requests,
labels: autocomplete.labels,
members: participants,
commands: autocomplete.commands(noteable, params[:type])
}
respond_to do |format|
format.json { render json: @suggestions }
end
end
def new_issue_address
return render_404 unless Gitlab::IncomingEmail.supports_issue_creation?
......
app/views/layouts/_init_auto_complete.html.haml
View file @ 1356e40f
......@@ -3,6 +3,14 @@
- if project
:javascript
GitLab.GfmAutoComplete.dataSource = "#{autocomplete_sources_namespace_project_path(project.namespace, project, type: noteable_type, type_id: params[:id])}"
GitLab.GfmAutoComplete.cachedData = undefined;
GitLab.GfmAutoComplete.setup();
gl.GfmAutoComplete.dataSources = {
emojis: "#{emojis_namespace_project_autocomplete_sources_path(project.namespace, project)}",
members: "#{members_namespace_project_autocomplete_sources_path(project.namespace, project, type: noteable_type, type_id: params[:id])}",
issues: "#{issues_namespace_project_autocomplete_sources_path(project.namespace, project)}",
mergeRequests: "#{merge_requests_namespace_project_autocomplete_sources_path(project.namespace, project)}",
labels: "#{labels_namespace_project_autocomplete_sources_path(project.namespace, project)}",
milestones: "#{milestones_namespace_project_autocomplete_sources_path(project.namespace, project)}",
commands: "#{commands_namespace_project_autocomplete_sources_path(project.namespace, project, type: noteable_type, type_id: params[:id])}"
};
gl.GfmAutoComplete.setup();
changelogs/unreleased/18435-autocomplete-is-not-performant.yml 0 → 100644
View file @ 1356e40f
---
title: Made comment autocomplete more performant and removed some loading bugs
merge_request: 6856
author:
config/routes/project.rb
View file @ 1356e40f
......@@ -11,6 +11,18 @@ constraints(ProjectUrlConstrainer.new) do
module: :projects,
as: :project) do
resources :autocomplete_sources, only: [] do
collection do
get 'emojis'
get 'members'
get 'issues'
get 'merge_requests'
get 'labels'
get 'milestones'
get 'commands'
end
end
#
# Templates
#
......@@ -316,7 +328,6 @@ constraints(ProjectUrlConstrainer.new) do
post :remove_export
post :generate_new_export
get :download_export
get :autocomplete_sources
get :activity
get :refs
put :new_issue_address
......
spec/features/participants_autocomplete_spec.rb
View file @ 1356e40f
require 'spec_helper'
feature 'Member autocomplete', feature: true do
include WaitForAjax
let(:project) { create(:project, :public) }
let(:user) { create(:user) }
let(:participant) { create(:user) }
......@@ -79,11 +81,10 @@ feature 'Member autocomplete', feature: true do
end
def open_member_suggestions
sleep 1
page.within('.new-note') do
sleep 1
find('#note_note').native.send_keys('@')
find('#note_note').send_keys('@')
end
wait_for_ajax
end
def visit_issue(project, issue)
......
spec/features/projects/gfm_autocomplete_load_spec.rb
View file @ 1356e40f
......@@ -10,12 +10,12 @@ describe 'GFM autocomplete loading', feature: true, js: true do
end
it 'does not load on project#show' do
expect(evaluate_script('GitLab.GfmAutoComplete.dataSource')).to eq('')
expect(evaluate_script('gl.GfmAutoComplete.dataSources')).to eq({})
end
it 'loads on new issue page' do
visit new_namespace_project_issue_path(project.namespace, project)
expect(evaluate_script('GitLab.GfmAutoComplete.dataSource')).not_to eq('')
expect(evaluate_script('gl.GfmAutoComplete.dataSources')).not_to eq({})
end
end
spec/routing/project_routing_spec.rb
View file @ 1356e40f
......@@ -80,10 +80,6 @@ describe 'project routing' do
expect(get('/gitlab/gitlabhq/edit')).to route_to('projects#edit', namespace_id: 'gitlab', id: 'gitlabhq')
end
it 'to #autocomplete_sources' do
expect(get('/gitlab/gitlabhq/autocomplete_sources')).to route_to('projects#autocomplete_sources', namespace_id: 'gitlab', id: 'gitlabhq')
end
describe 'to #show' do
context 'regular name' do
it { expect(get('/gitlab/gitlabhq')).to route_to('projects#show', namespace_id: 'gitlab', id: 'gitlabhq') }
......@@ -117,6 +113,21 @@ describe 'project routing' do
end
end
# emojis_namespace_project_autocomplete_sources_path GET /:project_id/autocomplete_sources/emojis(.:format) projects/autocomplete_sources#emojis
# members_namespace_project_autocomplete_sources_path GET /:project_id/autocomplete_sources/members(.:format) projects/autocomplete_sources#members
# issues_namespace_project_autocomplete_sources_path GET /:project_id/autocomplete_sources/issues(.:format) projects/autocomplete_sources#issues
# merge_requests_namespace_project_autocomplete_sources_path GET /:project_id/autocomplete_sources/merge_requests(.:format) projects/autocomplete_sources#merge_requests
# labels_namespace_project_autocomplete_sources_path GET /:project_id/autocomplete_sources/labels(.:format) projects/autocomplete_sources#labels
# milestones_namespace_project_autocomplete_sources_path GET /:project_id/autocomplete_sources/milestones(.:format) projects/autocomplete_sources#milestones
# commands_namespace_project_autocomplete_sources_path GET /:project_id/autocomplete_sources/commands(.:format) projects/autocomplete_sources#commands
describe Projects::AutocompleteSourcesController, 'routing' do
[:emojis, :members, :issues, :merge_requests, :labels, :milestones, :commands].each do |action|
it "to ##{action}" do
expect(get("/gitlab/gitlabhq/autocomplete_sources/#{action}")).to route_to("projects/autocomplete_sources##{action}", namespace_id: 'gitlab', project_id: 'gitlabhq')
end
end
end
# pages_project_wikis GET /:project_id/wikis/pages(.:format) projects/wikis#pages
# history_project_wiki GET /:project_id/wikis/:id/history(.:format) projects/wikis#history
# project_wikis POST /:project_id/wikis(.:format) projects/wikis#create
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7