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

Mention completion for issue editor. #3136

Merged
merged 7 commits into from Dec 11, 2017

Conversation

7 participants
@harryxu
Contributor

harryxu commented Dec 10, 2017

Mention completion on issue editor. @mention engine from tribute.

Demo:

New issue:

new issue

View/Edit issue:
view issue

@lafriks lafriks added this to the 1.4.0 milestone Dec 10, 2017

@codecov-io

This comment has been minimized.

codecov-io commented Dec 10, 2017

Codecov Report

Merging #3136 into master will increase coverage by 0.16%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3136      +/-   ##
==========================================
+ Coverage   34.57%   34.73%   +0.16%     
==========================================
  Files         276      276              
  Lines       39945    39948       +3     
==========================================
+ Hits        13809    13877      +68     
+ Misses      24155    24076      -79     
- Partials     1981     1995      +14
Impacted Files Coverage Δ
routers/repo/pull.go 34.34% <100%> (+0.08%) ⬆️
routers/repo/issue.go 31.9% <100%> (+0.12%) ⬆️
modules/indexer/repo.go 60.86% <0%> (-6.96%) ⬇️
models/issue_indexer.go 64.93% <0%> (-2.6%) ⬇️
models/repo_indexer.go 51.94% <0%> (-0.98%) ⬇️
models/error.go 33.02% <0%> (-0.62%) ⬇️
models/issue.go 44.82% <0%> (-0.21%) ⬇️
models/repo.go 41.47% <0%> (+0.18%) ⬆️
models/lfs.go 28.26% <0%> (+2.17%) ⬆️
modules/lfs/server.go 35.01% <0%> (+14.32%) ⬆️
... and 1 more

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 f2e20c8...a9aa351. Read the comment docs.

@lafriks

This comment has been minimized.

Member

lafriks commented Dec 10, 2017

Can you convert tribute css to less file and include into main gitea css? Then there would be no need for additional css include

@ethantkoenig

Mostly looks good, just a few minor things

@@ -351,6 +351,7 @@ func NewIssue(ctx *context.Context) {
ctx.Data["PageIsIssueList"] = true
ctx.Data["RequireHighlightJS"] = true
ctx.Data["RequireSimpleMDE"] = true
ctx.Data["RequireTribute"] = true

This comment has been minimized.

@ethantkoenig

ethantkoenig Dec 10, 2017

Member

I believe we also should require tribute in CompareAndPullRequest

@@ -58,6 +58,33 @@
{{if .RequireDropzone}}
<script src="{{AppSubUrl}}/vendor/plugins/dropzone/dropzone.js"></script>
{{end}}

This comment has been minimized.

@ethantkoenig

ethantkoenig Dec 10, 2017

Member

nit: delete blank line for consistency

],
noMatchTemplate: function () { return null },
menuItemTemplate: function (item) {
var user = item.original;

This comment has been minimized.

@ethantkoenig

ethantkoenig Dec 10, 2017

Member

nit: should the function body (lines 76-81) be indented?

@harryxu

This comment has been minimized.

Contributor

harryxu commented Dec 11, 2017

@lafriks I will check out later, but I see other JS plugin's css also included by this way.

@harryxu

This comment has been minimized.

Contributor

harryxu commented Dec 11, 2017

@lafriks

This comment has been minimized.

Member

lafriks commented Dec 11, 2017

@harryxu yes I know, this one has small css so it was just an idea, it's totally not required and not blocking this PR

@ethantkoenig

This comment has been minimized.

Member

ethantkoenig commented Dec 11, 2017

LGTM

@tboerger tboerger added lgtm/need 1 and removed lgtm/need 2 labels Dec 11, 2017

@lunny

This comment has been minimized.

Member

lunny commented Dec 11, 2017

LGTM

@tboerger tboerger added lgtm/done and removed lgtm/need 1 labels Dec 11, 2017

@lunny lunny merged commit 03ec35e into go-gitea:master Dec 11, 2017

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr the build was successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment