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

Add user search autocomplete #2108

Merged
merged 28 commits into from
Aug 30, 2018

Conversation

octavioamu
Copy link
Contributor

Description

Add autocomplete typeahead features for usernames
Fix select2 sizes

Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

UI

Testing
Refers/Fixes

Fixes: #2001

@@ -66,12 +66,14 @@ $(document).ready(function() {
if ($(this).hasClass('disabled'))
return;
loading_button($(this));

console.log('dentro')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon. (semi)

// get form data
var email = $('#email').val();
var github_url = $('#issueURL').val();
var from_name = $('#fromName').val();
var username = $('#username').val();
// var username = $('#username').val();
var username = $('.username-search').select2('data')[0].text

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected blank line after variable declarations. (newline-after-var)
Missing semicolon. (semi)

var username = $('#username').val();
// var username = $('#username').val();
var username = $('.username-search').select2('data')[0].text
console.log(username)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon. (semi)

url: '/api/v0.1/users_search/',
dataType: 'json',
delay: 250,
data: function (params) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected space before function parentheses. (space-before-function-paren)

dataType: 'json',
delay: 250,
data: function (params) {
var query = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected blank line after variable declarations. (newline-after-var)

minimumInputLength: 3,
escapeMarkup: function (markup) { return markup; }, // let our custom formatter work
templateResult: formatUser,
templateSelection: formatUserSelection,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected trailing comma. (comma-dangle)

// theme: "bootstrap4"
});

function formatUser (user) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected space before function parentheses. (space-before-function-paren)

return markup;
}

function formatUserSelection (user) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected space before function parentheses. (space-before-function-paren)

if (user.id) {
var selected = `<img class="rounded-circle" src="${user.avatar_url || static_url + 'v2/images/user-placeholder.png'}" width="20" height="20"/> ${user.text}`;
}
return selected || user.text;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'selected' used outside of binding context. (block-scoped-var)

else:
data = 'fail'
mimetype = 'application/json'
return HttpResponse(data, mimetype)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W292 no newline at end of file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't understand in my file is an empty last line

delay: 250,
data: function(params) {

var query = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected blank line after variable declarations. (newline-after-var)

data: function(params) {

var query = {
term: params.term,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected trailing comma. (comma-dangle)


var query = {
term: params.term,
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon. (semi)

},
processResults: function(data) {
return {
results: data,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected trailing comma. (comma-dangle)
Trailing spaces not allowed. (no-trailing-spaces)

},
placeholder: 'Search by username',
minimumInputLength: 3,
escapeMarkup: function(markup) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing spaces not allowed. (no-trailing-spaces)

minimumInputLength: 3,
escapeMarkup: function(markup) {

return markup;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing spaces not allowed. (no-trailing-spaces)


function formatUserSelection(user) {
if (user.id) {
var selected = `<img class="rounded-circle" src="${user.avatar_url || static_url + 'v2/images/user-placeholder.png'}" width="20" height="20"/> ${user.text}`;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'selected' used outside of binding context. (block-scoped-var)

if (user.id) {
var selected = `<img class="rounded-circle" src="${user.avatar_url || static_url + 'v2/images/user-placeholder.png'}" width="20" height="20"/> ${user.text}`;
} else {
var selected = user.text

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'selected' is already defined. (no-redeclare)
'selected' used outside of binding context. (block-scoped-var)
Missing semicolon. (semi)

} else {
var selected = user.text
}
return selected;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'selected' used outside of binding context. (block-scoped-var)

@thelostone-mc
Copy link
Member

@octavioamu could we

  • replace var with let to suppress the linting errors
  • fix the remaining linting errors
  • rebase & resolve conflicts

if (user.loading) {
return user.text;
}
let markup = `<div class="d-flex align-items-baseline">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected blank line after variable declarations. (newline-after-var)

}

function formatUserSelection(user) {
let selected;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected blank line after variable declarations. (newline-after-var)

if (user.id) {
selected = `<img class="rounded-circle" src="${user.avatar_url || static_url + 'v2/images/user-placeholder.png'}" width="20" height="20"/> ${user.text}`;
} else {
selected = user.text

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing semicolon. (semi)

@@ -1557,3 +1557,25 @@ def change_bounty(request, bounty_id):
'result': result
}
return TemplateResponse(request, 'bounty/change.html', params)


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W293 blank line contains whitespace

@codecov
Copy link

codecov bot commented Aug 27, 2018

Codecov Report

Merging #2108 into master will decrease coverage by 0.03%.
The diff coverage is 10%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2108      +/-   ##
==========================================
- Coverage   27.91%   27.87%   -0.04%     
==========================================
  Files         136      136              
  Lines       11130    11149      +19     
  Branches     1499     1502       +3     
==========================================
+ Hits         3107     3108       +1     
- Misses       7913     7931      +18     
  Partials      110      110
Impacted Files Coverage Δ
app/app/urls.py 84.09% <ø> (ø) ⬆️
app/dashboard/views.py 13.77% <10%> (-0.23%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5cd59db...c70e19e. Read the comment docs.


function formatUserSelection(user) {

let selected;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected blank line after variable declarations. (newline-after-var)

@@ -1,4 +1,4 @@
# -*- coding: utf-8 -*-
# -*- coding: utf-8 -*-

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E114 indentation is not a multiple of four (comment)
E116 unexpected indentation (comment)

@thelostone-mc
Copy link
Member

@octavioamu I fixed the errors 😂

screen shot 2018-08-28 at 5 08 20 pm

thelostone-mc
thelostone-mc previously approved these changes Aug 28, 2018
Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

results.append(profile_json)
data = json.dumps(results)
else:
data = 'fail'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when this block of code gets executed? Won't the formatUser fail? Secondly, if you are sending it with application/json, I think it is better to send something like {message: 'fail'}

{% trans "To Github Username" %}:
<input type="text" placeholder="@username" id="username" value="" style="max-width: 400px; margin-left: auto; margin-right: auto;">
<label for="">{% trans "To Github Username" %}:</label> <br>
<!-- <input type="text" placeholder="@username" id="username" value="" style="max-width: 400px; margin-left: auto; margin-right: auto;"> -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@octavioamu could we remove this if it's not being used ?

@octavioamu
Copy link
Contributor Author

Now looks like worked, not sure what bug is getting github but my branch was diff than this one and the changes wasn't being shown here, now all the changes appears. I was getting crazy....
@SaptakS I had changed the code, if a request is not ajax 404 page is shown and not a new page with a text message.
@thelostone-mc removed the commented code

@mbeacom
Copy link
Contributor

mbeacom commented Aug 28, 2018

@octavioamu Seems there is some latency picking up changes on GH today.

thelostone-mc
thelostone-mc previously approved these changes Aug 28, 2018
@thelostone-mc thelostone-mc merged commit 44cfa66 into gitcoinco:master Aug 30, 2018
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.

Add Ability to Select a Contributor via autocomplete to Tip
6 participants