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.

Sign up for GitHub

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

Jump to bottom

Added progressbar for issues (#1146). #3171

Merged
merged 13 commits into from
Jan 3, 2018
Merged

Added progressbar for issues (#1146). #3171

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
904ee41
Added progressbar for issues (#1146).
modmew8 Dec 12, 2017
d9f4e82
Updated the generated index.css.
modmew8 Dec 12, 2017
4ca3005
Merge pull request #1 from go-gitea/master
modmew8 Dec 13, 2017
5d78b7f
Merge pull request #2 from modmew8/master
modmew8 Dec 13, 2017
c220a27
Removed stored progress percentage and changed it to css calc. Also a…
modmew8 Dec 13, 2017
265636c
Removed unnecessary blanks.
modmew8 Dec 13, 2017
7ce012b
Formatted the files correctly, fmt-check terminates now without errors.
modmew8 Dec 13, 2017
74adf36
Merge branch 'master' of https://github.com/go-gitea/gitea into go-gi…
modmew8 Jan 2, 2018
890349e
Merge branch 'go-gitea-master' into progressbarForIssues
modmew8 Jan 2, 2018
2fdeeb5
Merge pull request #4 from go-gitea/master
modmew8 Jan 2, 2018
7088869
Merge pull request #5 from modmew8/master
modmew8 Jan 2, 2018
3b7a161
Removed variables, made computing the tasks on demand with precompile…
modmew8 Jan 2, 2018
ae0d1df
Merge branch 'master' into progressbarForIssues
lunny Jan 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions models/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package models
import (
"fmt"
"path"
"regexp"
"sort"
"strings"

Expand All @@ -31,9 +32,11 @@ type Issue struct {
Title string `xorm:"name"`
Content string `xorm:"TEXT"`
RenderedContent string `xorm:"-"`
Labels []*Label `xorm:"-"`
MilestoneID int64 `xorm:"INDEX"`
Milestone *Milestone `xorm:"-"`
Tasks int
Copy link
Member

Choose a reason for hiding this comment

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

If we're adding new columns to the issue table, then we need a migration.

However, I think the case could be made that we shouldn't store tasks/tasksDone in the database, but instead just calculate them from the Content field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about leaving the parsing to the clients? Then we would not have to store anything on the server, and the only files being changed would be the template files (and the css rules). Since it is just a cosmetic addition, the clients could determine the progress themselves upon viewing the issue list - but the server has to provide the content for the clients to parse (additional network load).

I would realize this as a javascript function in the issue list and in the user dashboard issue list.

Copy link
Member

Choose a reason for hiding this comment

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

@ethantkoenig I don't think so. If the page has 10 issues. We have to parse all the 10 issues' markdowns on the browser one time. And also we need add a javascript markdown library. I don't think this is a good idea.

Copy link
Member

Choose a reason for hiding this comment

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

@lunny We don't need to render the issue content as markdown, we just need to look at the raw string (which we load from the DB anyway). The regexps in this PR would not work if the issue contents were rendered.

Copy link
Member

Choose a reason for hiding this comment

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

@ethantkoenig and another thing, we didn't transfer issue content to client on issue list. If you want client to parse that. We have to transfer the contents. This will also reduce the load speed of the page.

Copy link
Member

Choose a reason for hiding this comment

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

I am not suggesting we send the issues' content to the client. That was @modmew8's comment.

I am proposing that we calculate the number of tasks server-side; this can be computed easily with regexps, and does not require rendering the contents as markdown. If our regexps are pre-compiled, I don't think this will be very expensive, since (as I mentioned above) we already load the contents from the DB.

I am hesitant to add new columns to the issue table because I'm afraid that if we will add new columns for every new feature, our schemas will eventually become very complicated and confusing.

Tasksdone int
Copy link
Member

Choose a reason for hiding this comment

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

TasksDone

Labels []*Label `xorm:"-"`
MilestoneID int64 `xorm:"INDEX"`
Milestone *Milestone `xorm:"-"`
Priority int
AssigneeID int64 `xorm:"INDEX"`
Assignee *User `xorm:"-"`
Expand Down Expand Up @@ -740,7 +743,16 @@ func AddDeletePRBranchComment(doer *User, repo *Repository, issueID int64, branc
func (issue *Issue) ChangeContent(doer *User, content string) (err error) {
oldContent := issue.Content
issue.Content = content
if err = UpdateIssueCols(issue, "content"); err != nil {

regExpTasks := regexp.MustCompile(`(^\s*-\s\[[\sx]\]\s)|(\n\s*-\s\[[\sx]\]\s)`)
Copy link
Member

Choose a reason for hiding this comment

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

This is duplicated code, please consider refactoring.

Also, it would be nice if we didn't have to compile the regexp every time.

tasksMatches := regExpTasks.FindAllStringIndex(content, -1)
issue.Tasks = len(tasksMatches)

regExpTasksdone := regexp.MustCompile(`(^\s*-\s\[[x]\]\s)|(\n\s*-\s\[[x]\]\s)`)
tasksdoneMatches := regExpTasksdone.FindAllStringIndex(content, -1)
issue.Tasksdone = len(tasksdoneMatches)

if err = UpdateIssueCols(issue, "content", "tasks", "tasksdone"); err != nil {
return fmt.Errorf("UpdateIssueCols: %v", err)
}

Expand Down
2 changes: 1 addition & 1 deletion public/css/index.css
View file
Open in desktop

Large diffs are not rendered by default.

14 changes: 14 additions & 0 deletions public/less/_repository.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1464,6 +1464,20 @@
.desc {
padding-top: 5px;
color: #999;
.progress-bar {
width: 80px;
height: 6px;
display: inline-block;
background-color: #eee;
overflow: hidden;
border-radius: 3px;
vertical-align: middle !important;
.progress {
background-color: #ccc;
display: block;
height: 100%;
}
}
a.milestone {
padding-left: 5px;
color: #999!important;
Expand Down
11 changes: 11 additions & 0 deletions routers/repo/issue.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"fmt"
"io"
"io/ioutil"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -454,6 +455,14 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) {
return
}

regExpTasks := regexp.MustCompile(`(^\s*-\s\[[\sx]\]\s)|(\n\s*-\s\[[\sx]\]\s)`)
tasksMatches := regExpTasks.FindAllStringIndex(form.Content, -1)
tasks := len(tasksMatches)

regExpTasksdone := regexp.MustCompile(`(^\s*-\s\[[x]\]\s)|(\n\s*-\s\[[x]\]\s)`)
tasksdoneMatches := regExpTasksdone.FindAllStringIndex(form.Content, -1)
tasksdone := len(tasksdoneMatches)

issue := &models.Issue{
RepoID: repo.ID,
Title: form.Title,
Expand All @@ -463,6 +472,8 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) {
AssigneeID: assigneeID,
Content: form.Content,
Ref: form.Ref,
Tasks: tasks,
Tasksdone: tasksdone,
}
if err := models.NewIssue(repo, issue, labelIDs, attachments); err != nil {
ctx.Handle(500, "NewIssue", err)
Expand Down
3 changes: 3 additions & 0 deletions templates/repo/issue/list.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@

<p class="desc">
{{$.i18n.Tr "repo.issues.opened_by" $timeStr .Poster.HomeLink .Poster.Name | Safe}}
{{if gt .Tasks 0}}
<span class="octicon octicon-checklist"></span> {{.Tasksdone}} / {{.Tasks}} <span class="progress-bar"><span class="progress" style="width:calc(100% * {{.Tasksdone}} / {{.Tasks}});"></span></span>
{{end}}
{{if .Milestone}}
<a class="milestone" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&state={{$.State}}&labels={{$.SelectLabels}}&milestone={{.Milestone.ID}}&assignee={{$.AssigneeID}}">
<span class="octicon octicon-milestone"></span> {{.Milestone.Name}}
Expand Down
3 changes: 3 additions & 0 deletions templates/user/dashboard/issues.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@
<img class="ui avatar image" src="{{.Assignee.RelAvatarLink}}">
</a>
{{end}}
{{if gt .Tasks 0}}
<span class="octicon octicon-checklist"></span> {{.Tasksdone}} / {{.Tasks}} <span class="progress-bar"><span class="progress" style="width:calc(100% * {{.Tasksdone}} / {{.Tasks}});"></span></span>
{{end}}
</p>
</li>
{{end}}
Expand Down