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

Use Semantic UI's Search component for user and repo search #2636

Merged
merged 9 commits into from
Oct 3, 2017

Conversation

harryxu
Copy link
Contributor

@harryxu harryxu commented Oct 1, 2017

Current user and repo search input is not support keyboard navigation and must use mouse click to select item.

So I use Semantic UI's Search component replace the jquery ajax code and html generating code, for improve the UI experience.

Here is the screenshots:

New user search

New repo search

Old user search (Can only use mouse select):

@codecov-io
Copy link

codecov-io commented Oct 1, 2017

Codecov Report

Merging #2636 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2636   +/-   ##
=======================================
  Coverage   27.11%   27.11%           
=======================================
  Files          86       86           
  Lines       17062    17062           
=======================================
  Hits         4627     4627           
  Misses      11757    11757           
  Partials      678      678

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 a04718a...916ad17. Read the comment docs.

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 1, 2017
$searchRepoBox.search({
minCharacters: 2,
apiSettings: {
url: suburl + '/api/v1/repos/search?q={query}',
Copy link
Member

Choose a reason for hiding this comment

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

uid query parameter is missing.

Copy link
Contributor Author

@harryxu harryxu Oct 2, 2017

Choose a reason for hiding this comment

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

@daviian Fixed.

Copy link
Member

@ethantkoenig ethantkoenig left a comment

Choose a reason for hiding this comment

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

Looks good! Just one nit.

});
hideWhenLostFocus('#search-user-box .results', '#search-user-box');
}

// FIXME: merge common parts in two functions
Copy link
Member

Choose a reason for hiding this comment

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

nit: remove this comment now that it is stale

@daviian
Copy link
Member

daviian commented Oct 2, 2017

Please remove comment as @ethantkoenig mentioned, otherwise LGTM

@tboerger tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 2, 2017
@harryxu
Copy link
Contributor Author

harryxu commented Oct 2, 2017

@ethantkoenig @daviian Done. 😃

@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Oct 2, 2017
@lunny lunny added this to the 1.3.0 milestone Oct 2, 2017
@lunny lunny added the type/enhancement An improvement of existing functionality label Oct 2, 2017
@ethantkoenig
Copy link
Member

LGTM

@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 2, 2017
@lafriks
Copy link
Member

lafriks commented Oct 2, 2017

Make LG-TM work again

@lafriks lafriks merged commit b3cfa5a into go-gitea:master Oct 3, 2017
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants