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

Expandable commit bodies #2980

Merged
merged 21 commits into from Nov 30, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 38 additions & 0 deletions modules/templates/helper.go
Expand Up @@ -110,6 +110,9 @@ func NewFuncMap() []template.FuncMap {
},
"RenderCommitMessage": RenderCommitMessage,
"RenderCommitMessageLink": RenderCommitMessageLink,
"RenderCommitBody": RenderCommitBody,
"RenderCommitBodyLink": RenderCommitBodyLink,
"IsMultilineCommitMessage": IsMultilineCommitMessage,
"ThemeColorMetaTag": func() string {
return setting.UI.ThemeColorMetaTag
},
Expand Down Expand Up @@ -278,6 +281,41 @@ func renderCommitMessage(msg string, opts markup.RenderIssueIndexPatternOptions)
return template.HTML(msgLines[0])
}

func RenderCommitBody(msg, urlPrefix string, metas map[string]string) template.HTML {
return renderCommitBody(msg, markup.RenderIssueIndexPatternOptions{
URLPrefix: urlPrefix,
Metas: metas,
})
}

func RenderCommitBodyLink(msg, urlPrefix string, urlDefault string, metas map[string]string) template.HTML {
Copy link
Member

Choose a reason for hiding this comment

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

This function does not seems to be used anywhere, so should be dropped

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! I just copied the RenderCommit functions and changed them a tiny bit.

return renderCommitMessage(msg, markup.RenderIssueIndexPatternOptions{
DefaultURL: urlDefault,
URLPrefix: urlPrefix,
Metas: metas,
})
}

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

func IsMultilineCommitMessage(msg string) bool {
msgLines := strings.Split(strings.TrimSpace(msg), "\n")
Copy link
Member

Choose a reason for hiding this comment

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

just do

return strings.Count(strings.TrimSpace(msg), "\n") > 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sweet, I was looking for a better way to do that.

if len(msgLines) > 1 {
return true
} else {
return false
}
}

// Actioner describes an action
type Actioner interface {
GetOpType() models.ActionType
Expand Down
4 changes: 4 additions & 0 deletions public/js/index.js
Expand Up @@ -2016,3 +2016,7 @@ function initFilterBranchTagDropdown(selector) {
});
});
}

$(".commit-button").click(function() {
$(this).next().toggle();
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a JS expert, but this seems fragile. What if we add a DOM element between the commit button and the commit body? Perhaps it would be better to select for the commit-body class?

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 tried doing

$(".commit-button").click(function() {
    $("commit-body").toggle();

But that opens and closes all of the commit messages, not just the one selected. All the documentation I could find from both JQuery themselves and online used this method, otherwise I think we'd have to use one of those JQuery-Toggle libraries.

Copy link
Member

Choose a reason for hiding this comment

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

Try:

$(".commit-button").on('click', function() {
  $(this).parent().find('.commit-body').toggle();
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll give it a go later today.

});
4 changes: 4 additions & 0 deletions templates/repo/commits_table.tmpl
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>
<p class="commit-body" style="display: none;">{{RenderCommitBody .Message $.RepoLink $.Repository.ComposeMetas}}</span>
{{end}}
{{template "repo/commit_status" .Status}}
</td>
<td class="grey text right aligned">{{TimeSince .Author.When $.Lang}}</td>
Expand Down