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

Expandable commit bodies #2980

Merged
merged 21 commits into from
Nov 30, 2017
Merged

Expandable commit bodies #2980

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
f4a48d2
Initial working state of expandable commit bodies
sondr3 Nov 26, 2017
67fde4d
Fix all commits having showing button for multiline commits
sondr3 Nov 26, 2017
524f957
Refactor checking multiline messages method
sondr3 Nov 26, 2017
053cc1d
Force newlines with <br> in commit body
sondr3 Nov 26, 2017
7afb927
Show multiple lines in the list view of repositories
sondr3 Nov 26, 2017
23b92b8
Fixed proper newlines and minor refactor
sondr3 Nov 26, 2017
7f25cd8
Fix commit list messages jumping around
sondr3 Nov 26, 2017
65e6a19
Fix indentation in view_list.tmpl
sondr3 Nov 27, 2017
ed572c2
Use vertical-align: baseline instead of top
sondr3 Nov 27, 2017
3767880
Refactor commit button toggle function
sondr3 Nov 27, 2017
2856f9e
Remove RenderCommitBodyLink function
sondr3 Nov 27, 2017
76e640d
Add comments
sondr3 Nov 27, 2017
f325cf2
Add newline at the end of _repository.less
sondr3 Nov 27, 2017
a452635
Fix long commit bodies not properly wrapping inside <pre>
sondr3 Nov 27, 2017
a47c4c1
Don't split on double newlines
sondr3 Nov 27, 2017
8f02f61
Show the commit body in commit view
sondr3 Nov 27, 2017
63fec45
Update stylesheets
sondr3 Nov 27, 2017
e521e8a
Merge branch 'master' into commit-message
lafriks Nov 27, 2017
f55878c
Add/fix comments and run make fmt
sondr3 Nov 27, 2017
083af01
Fix spaces not being tabs
sondr3 Nov 29, 2017
f7592bb
Merge branch 'master' into commit-message
lunny Nov 30, 2017
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
29 changes: 27 additions & 2 deletions modules/templates/helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,10 @@ func NewFuncMap() []template.FuncMap {
"EscapePound": func(str string) string {
return strings.NewReplacer("%", "%25", "#", "%23", " ", "%20", "?", "%3F").Replace(str)
},
"RenderCommitMessage": RenderCommitMessage,
"RenderCommitMessageLink": RenderCommitMessageLink,
"RenderCommitMessage": RenderCommitMessage,
"RenderCommitMessageLink": RenderCommitMessageLink,
"RenderCommitBody": RenderCommitBody,
"IsMultilineCommitMessage": IsMultilineCommitMessage,
"ThemeColorMetaTag": func() string {
return setting.UI.ThemeColorMetaTag
},
Expand Down Expand Up @@ -278,6 +280,29 @@ func renderCommitMessage(msg string, opts markup.RenderIssueIndexPatternOptions)
return template.HTML(msgLines[0])
}

// RenderCommitBody extracts the body of a commit message without its title.
func RenderCommitBody(msg, urlPrefix string, metas map[string]string) template.HTML {
return renderCommitBody(msg, markup.RenderIssueIndexPatternOptions{
URLPrefix: urlPrefix,
Metas: metas,
})
}

func renderCommitBody(msg string, opts markup.RenderIssueIndexPatternOptions) template.HTML {
cleanMsg := template.HTMLEscapeString(msg)
fullMessage := string(markup.RenderIssueIndexPattern([]byte(cleanMsg), opts))
body := strings.Split(strings.TrimSpace(fullMessage), "\n")
if len(body) == 0 {
return template.HTML("")
}
return template.HTML(strings.Join(body[1:], "\n"))
Copy link
Member

@ethantkoenig ethantkoenig Nov 27, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I think we should differentiate between single and double new-lines, since a single newline may just be wrapping a long line, but a double newline typically indicates the start of a new paragraph. In fact, I think the best solution is just to render newlines as they appear in the commit message (this is what Github does).

Also, note that newlines in a template.HTML will not be rendered as newlines. The somewhat hacky way to fix this problem is to replace newlines with <br>. (the crossed-out part is incorrect, ignore it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I initially tried joining on <br> but then I looked at Github and saw that they wrapped their commit messages in <pre>. However, if that's the case I think the method needs to work differently because it already splits on \n and then I discard the first element and join the others. It seems to render correctly, but I'll see what I can figure out.

}

// IsMultilineCommitMessage checks to see if a commit message contains multiple lines.
func IsMultilineCommitMessage(msg string) bool {
return strings.Count(strings.TrimSpace(msg), "\n") > 1
}

// Actioner describes an action
type Actioner interface {
GetOpType() models.ActionType
Expand Down
2 changes: 1 addition & 1 deletion public/css/index.css
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 4 additions & 0 deletions public/js/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2016,3 +2016,7 @@ function initFilterBranchTagDropdown(selector) {
});
});
}

$(".commit-button").click(function() {
$(this).parent().find('.commit-body').toggle();
});
8 changes: 8 additions & 0 deletions public/less/_repository.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1606,3 +1606,11 @@
}
}
}

.commit-list {
vertical-align: baseline;
}

.commit-body {
white-space: pre-wrap;
}
6 changes: 5 additions & 1 deletion templates/repo/commits_table.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
<th class="three wide right aligned">{{.i18n.Tr "repo.commits.date"}}</th>
</tr>
</thead>
<tbody>
<tbody class="commit-list">
{{ $r:= List .Commits}}
{{range $r}}
<tr>
Expand Down Expand Up @@ -61,6 +61,10 @@
</td>
<td class="message collapsing">
<span class="has-emoji{{if gt .ParentCount 1}} grey text{{end}}">{{RenderCommitMessage .Summary $.RepoLink $.Repository.ComposeMetas}}</span>
{{if IsMultilineCommitMessage .Message}}
<button class="basic compact mini ui icon button commit-button"><i class="ellipsis horizontal icon"></i></button>
<pre class="commit-body" style="display: none;">{{RenderCommitBody .Message $.RepoLink $.Repository.ComposeMetas}}</pre>
Copy link
Member

Choose a reason for hiding this comment

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

There is inline CSS here, could you change it out with a CSS class?
Also this requires JS which means if someone has JS disabled then they won't be able to see the long commit message (perhaps default to open when JS is disabled?)

Copy link
Member

Choose a reason for hiding this comment

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

Or perhaps hide the button if the user doesn't have JS is another option

Copy link
Member

Choose a reason for hiding this comment

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

@techknowlogick I think for this inline style is ok

Copy link
Contributor Author

@sondr3 sondr3 Nov 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I agree it's not the prettiest, but the toggle function changes the style inline so I thought it was a decent compromise. Regarding what to do when JS is disabled, I'll leave that to the Gitea maintainers, I think Gitea depends on JS enough that if you disable it you're on your own.

{{end}}
{{template "repo/commit_status" .Status}}
</td>
<td class="grey text right aligned">{{TimeSince .Author.When $.Lang}}</td>
Expand Down
3 changes: 3 additions & 0 deletions templates/repo/diff/page.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
{{.i18n.Tr "repo.diff.browse_source"}}
</a>
<h3>{{RenderCommitMessage .Commit.Message $.RepoLink $.Repository.ComposeMetas}}{{template "repo/commit_status" .CommitStatus}}</h3>
{{if IsMultilineCommitMessage .Commit.Message}}
<pre class="commit-body">{{RenderCommitBody .Commit.Message $.RepoLink $.Repository.ComposeMetas}}</pre>
{{end}}
</div>
<div class="ui attached info segment {{if .Commit.Signature}} isSigned {{if .Verification.Verified }} isVerified {{end}}{{end}}">
{{if .Author}}
Expand Down
6 changes: 5 additions & 1 deletion templates/repo/view_list.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<table id="repo-files-table" class="ui fixed single line table">
<thead>
<tr>
<tr class="commit-list">
<th class="four wide">
{{if .LatestCommitUser}}
<img class="ui avatar image img-12" src="{{.LatestCommitUser.RelAvatarLink}}" />
Expand Down Expand Up @@ -28,6 +28,10 @@
{{end}}
</a>
<span class="grey has-emoji">{{RenderCommitMessage .LatestCommit.Summary .RepoLink $.Repository.ComposeMetas}}
{{if IsMultilineCommitMessage .LatestCommit.Message}}
<button class="basic compact mini ui icon button commit-button"><i class="ellipsis horizontal icon"></i></button>
<pre class="commit-body" style="display: none;">{{RenderCommitBody .LatestCommit.Message $.RepoLink $.Repository.ComposeMetas}}</pre>
Copy link
Member

Choose a reason for hiding this comment

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

same as above regarding inline CSS

{{end}}
{{template "repo/commit_status" .LatestCommitStatus}}</span>
</th>
<th class="nine wide">
Expand Down