Improve labels-list rendering (#34846)

Make labels list use consistent gap

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
silverwind 2025-06-27 17:12:25 +02:00 committed by GitHub
parent aa9d86745a
commit 1e50cec0b3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
19 changed files with 185 additions and 115 deletions

View file

@ -42,6 +42,8 @@ func HTMLFormat(s template.HTML, rawArgs ...any) template.HTML {
// for most basic types (including template.HTML which is safe), just do nothing and use it
case string:
args[i] = template.HTMLEscapeString(v)
case template.URL:
args[i] = template.HTMLEscapeString(string(v))
case fmt.Stringer:
args[i] = template.HTMLEscapeString(v.String())
default:

View file

@ -10,6 +10,15 @@ import (
"github.com/stretchr/testify/assert"
)
type testStringer struct{}
func (t testStringer) String() string {
return "&StringMethod"
}
func TestHTMLFormat(t *testing.T) {
assert.Equal(t, template.HTML("<a>&lt; < 1</a>"), HTMLFormat("<a>%s %s %d</a>", "<", template.HTML("<"), 1))
assert.Equal(t, template.HTML("%!s(<nil>)"), HTMLFormat("%s", nil))
assert.Equal(t, template.HTML("&lt;&gt;"), HTMLFormat("%s", template.URL("<>")))
assert.Equal(t, template.HTML("&amp;StringMethod &amp;StringMethod"), HTMLFormat("%s %s", testStringer{}, &testStringer{}))
}

View file

@ -122,8 +122,23 @@ func (ut *RenderUtils) RenderIssueSimpleTitle(text string) template.HTML {
return ret
}
// RenderLabel renders a label
func (ut *RenderUtils) RenderLabelWithLink(label *issues_model.Label, link any) template.HTML {
var attrHref template.HTML
switch link.(type) {
case template.URL, string:
attrHref = htmlutil.HTMLFormat(`href="%s"`, link)
default:
panic(fmt.Sprintf("unexpected type %T for link", link))
}
return ut.renderLabelWithTag(label, "a", attrHref)
}
func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML {
return ut.renderLabelWithTag(label, "span", "")
}
// RenderLabel renders a label
func (ut *RenderUtils) renderLabelWithTag(label *issues_model.Label, tagName, tagAttrs template.HTML) template.HTML {
locale := ut.ctx.Value(translation.ContextKey).(translation.Locale)
var extraCSSClasses string
textColor := util.ContrastColor(label.Color)
@ -137,8 +152,8 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML {
if labelScope == "" {
// Regular label
return htmlutil.HTMLFormat(`<div class="ui label %s" style="color: %s !important; background-color: %s !important;" data-tooltip-content title="%s">%s</div>`,
extraCSSClasses, textColor, label.Color, descriptionText, ut.RenderEmoji(label.Name))
return htmlutil.HTMLFormat(`<%s %s class="ui label %s" style="color: %s !important; background-color: %s !important;" data-tooltip-content title="%s"><span class="gt-ellipsis">%s</span></%s>`,
tagName, tagAttrs, extraCSSClasses, textColor, label.Color, descriptionText, ut.RenderEmoji(label.Name), tagName)
}
// Scoped label
@ -152,7 +167,7 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML {
// Ensure we add the same amount of contrast also near 0 and 1.
darken := contrast + math.Max(luminance+contrast-1.0, 0.0)
lighten := contrast + math.Max(contrast-luminance, 0.0)
// Compute factor to keep RGB values proportional.
// Compute the factor to keep RGB values proportional.
darkenFactor := math.Max(luminance-darken, 0.0) / math.Max(luminance, 1.0/255.0)
lightenFactor := math.Min(luminance+lighten, 1.0) / math.Max(luminance, 1.0/255.0)
@ -173,26 +188,29 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML {
if label.ExclusiveOrder > 0 {
// <scope> | <label> | <order>
return htmlutil.HTMLFormat(`<span class="ui label %s scope-parent" data-tooltip-content title="%s">`+
return htmlutil.HTMLFormat(`<%s %s class="ui label %s scope-parent" data-tooltip-content title="%s">`+
`<div class="ui label scope-left" style="color: %s !important; background-color: %s !important">%s</div>`+
`<div class="ui label scope-middle" style="color: %s !important; background-color: %s !important">%s</div>`+
`<div class="ui label scope-right">%d</div>`+
`</span>`,
`</%s>`,
tagName, tagAttrs,
extraCSSClasses, descriptionText,
textColor, scopeColor, scopeHTML,
textColor, itemColor, itemHTML,
label.ExclusiveOrder)
label.ExclusiveOrder,
tagName)
}
// <scope> | <label>
return htmlutil.HTMLFormat(`<span class="ui label %s scope-parent" data-tooltip-content title="%s">`+
return htmlutil.HTMLFormat(`<%s %s class="ui label %s scope-parent" data-tooltip-content title="%s">`+
`<div class="ui label scope-left" style="color: %s !important; background-color: %s !important">%s</div>`+
`<div class="ui label scope-right" style="color: %s !important; background-color: %s !important">%s</div>`+
`</span>`,
`</%s>`,
tagName, tagAttrs,
extraCSSClasses, descriptionText,
textColor, scopeColor, scopeHTML,
textColor, itemColor, itemHTML,
)
tagName)
}
// RenderEmoji renders html text with emoji post processors
@ -235,7 +253,8 @@ func (ut *RenderUtils) RenderLabels(labels []*issues_model.Label, repoLink strin
if label == nil {
continue
}
htmlCode += fmt.Sprintf(`<a href="%s?labels=%d">%s</a>`, baseLink, label.ID, ut.RenderLabel(label))
link := fmt.Sprintf("%s?labels=%d", baseLink, label.ID)
htmlCode += string(ut.RenderLabelWithLink(label, template.URL(link)))
}
htmlCode += "</span>"
return template.HTML(htmlCode)

View file

@ -205,6 +205,17 @@ func TestRenderLabels(t *testing.T) {
issue = &issues.Issue{IsPull: true}
expected = `/owner/repo/pulls?labels=123`
assert.Contains(t, ut.RenderLabels([]*issues.Label{label}, "/owner/repo", issue), expected)
expectedLabel := `<a href="&lt;&gt;" class="ui label " style="color: #fff !important; background-color: label-color !important;" data-tooltip-content title=""><span class="gt-ellipsis">label-name</span></a>`
assert.Equal(t, expectedLabel, string(ut.RenderLabelWithLink(label, "<>")))
assert.Equal(t, expectedLabel, string(ut.RenderLabelWithLink(label, template.URL("<>"))))
label = &issues.Label{ID: 123, Name: "</>", Exclusive: true}
expectedLabel = `<a href="" class="ui label scope-parent" data-tooltip-content title=""><div class="ui label scope-left" style="color: #fff !important; background-color: #000000 !important">&lt;</div><div class="ui label scope-right" style="color: #fff !important; background-color: #000000 !important">&gt;</div></a>`
assert.Equal(t, expectedLabel, string(ut.RenderLabelWithLink(label, "")))
label = &issues.Label{ID: 123, Name: "</>", Exclusive: true, ExclusiveOrder: 1}
expectedLabel = `<a href="" class="ui label scope-parent" data-tooltip-content title=""><div class="ui label scope-left" style="color: #fff !important; background-color: #000000 !important">&lt;</div><div class="ui label scope-middle" style="color: #fff !important; background-color: #000000 !important">&gt;</div><div class="ui label scope-right">1</div></a>`
assert.Equal(t, expectedLabel, string(ut.RenderLabelWithLink(label, "")))
}
func TestUserMention(t *testing.T) {

View file

@ -10,8 +10,8 @@
</a>
</div>
<div class="flex-item">
<div class="flex-item-icon">
{{svg "octicon-tag" 16}}
<div class="flex-item-leading">
<div class="tw-mt-1">{{svg "octicon-tag" 16}}</div>
</div>
<div class="flex-item-main">
<div class="flex-item-header">

View file

@ -63,16 +63,21 @@
{{if or .Labels .Assignees}}
<div class="issue-card-bottom">
<div class="labels-list">
{{range .Labels}}
<a target="_blank" href="{{$.Issue.Repo.Link}}/issues?labels={{.ID}}">{{ctx.RenderUtils.RenderLabel .}}</a>
{{/* the labels container must always be present, to help the assignees list right-aligned */}}
<div class="issue-card-bottom-part labels-list">
{{range $label := .Labels}}
{{$link := print $.Issue.Repo.Link "/issues"}}
{{$link = QueryBuild $link "labels" $label.ID}}
{{ctx.RenderUtils.RenderLabelWithLink $label $link}}
{{end}}
</div>
<div class="issue-card-assignees">
{{if .Assignees}}
<div class="issue-card-bottom-part tw-justify-end">
{{range .Assignees}}
<a target="_blank" href="{{.HomeLink}}" data-tooltip-content="{{ctx.Locale.Tr "repo.projects.column.assigned_to"}} {{.Name}}">{{ctx.AvatarUtils.Avatar . 28}}</a>
<a target="_blank" href="{{.HomeLink}}" data-tooltip-content="{{ctx.Locale.Tr "repo.projects.column.assigned_to"}} {{.Name}}">{{ctx.AvatarUtils.Avatar . 24}}</a>
{{end}}
</div>
{{end}}
</div>
{{end}}
{{end}}

View file

@ -26,7 +26,7 @@
<div class="divider"></div>
{{end}}
<ul class="issue-label-list">
<ul class="issue-label-list muted-links">
{{$canEditLabel := and (not $.PageIsOrgSettingsLabels) (not $.Repository.IsArchived) (or $.CanWriteIssues $.CanWritePulls)}}
{{$canEditLabel = or $canEditLabel $.PageIsOrgSettingsLabels}}
{{range .Labels}}
@ -42,9 +42,8 @@
<a class="open-issues" href="{{$.RepoLink}}/issues?labels={{.ID}}">{{svg "octicon-issue-opened"}} {{ctx.Locale.Tr "repo.issues.label_open_issues" .NumOpenIssues}}</a>
{{end}}
</div>
<div class="label-operation tw-flex">
<div class="label-operation">
{{template "repo/issue/labels/label_archived" .}}
<div class="tw-flex tw-ml-auto">
{{if $canEditLabel}}
<a class="edit-label-button" href="#"
data-label-id="{{.ID}}" data-label-name="{{.Name}}" data-label-color="{{.Color}}"
@ -57,7 +56,6 @@
data-modal-confirm-content="{{ctx.Locale.Tr "repo.issues.label_deletion_desc"}}"
>{{svg "octicon-trash"}} {{ctx.Locale.Tr "repo.issues.label_delete"}}</a>
{{end}}
</div>
</div>
</li>
{{end}}

View file

@ -27,7 +27,7 @@
</div>
</div>
</div>
<div class="ui list muted-links flex-items-block tw-flex tw-flex-col tw-gap-2">
<div class="ui relaxed list muted-links flex-items-block">
<span class="item empty-list {{if $issueAssignees}}tw-hidden{{end}}">{{ctx.Locale.Tr "repo.issues.new.no_assignees"}}</span>
{{range $issueAssignees}}
<a class="item" href="{{$pageMeta.RepoLink}}/{{if $pageMeta.IsPullRequest}}pulls{{else}}issues{{end}}?assignee={{.ID}}">

View file

@ -43,7 +43,7 @@
</div>
</div>
<div class="ui list labels-list tw-my-2 tw-flex tw-gap-2">
<div class="ui list labels-list">
<span class="item empty-list {{if $data.SelectedLabelIDs}}tw-hidden{{end}}">{{ctx.Locale.Tr "repo.issues.new.no_label"}}</span>
{{range $data.AllLabels}}
{{if .IsChecked}}

View file

@ -43,7 +43,7 @@
</div>
</div>
<div class="ui relaxed list flex-items-block tw-my-4">
<div class="ui relaxed list flex-items-block">
<span class="item empty-list {{if or $data.OriginalReviews $data.CurrentPullReviewers}}tw-hidden{{end}}">
{{ctx.Locale.Tr "repo.issues.new.no_reviewers"}}
</span>

View file

@ -169,7 +169,7 @@
</div>
{{else if eq .Type 7}}
{{if or .AddedLabels .RemovedLabels}}
<div class="timeline-item event" id="{{.HashTag}}">
<div class="timeline-item event with-labels-list-inline" id="{{.HashTag}}">
<span class="badge">{{svg "octicon-tag"}}</span>
{{template "shared/user/avatarlink" dict "user" .Poster}}
<span class="text grey muted-links">

View file

@ -3,11 +3,14 @@
{{range .Issues}}
<div class="flex-item">
<div class="flex-item-icon">
{{if $.CanWriteIssuesOrPulls}}
<input type="checkbox" autocomplete="off" class="issue-checkbox tw-mr-4" data-issue-id={{.ID}} aria-label="{{ctx.Locale.Tr "repo.issues.action_check"}} &quot;{{.Title}}&quot;">
{{end}}
{{template "shared/issueicon" .}}
<div class="flex-item-leading">
{{/* using some tw helpers is the only way to align the checkbox */}}
<div class="flex-text-inline tw-mt-[2px]">
{{if $.CanWriteIssuesOrPulls}}
<input type="checkbox" autocomplete="off" class="issue-checkbox tw-mr-[14px]" data-issue-id={{.ID}} aria-label="{{ctx.Locale.Tr "repo.issues.action_check"}} &quot;{{.Title}}&quot;">
{{end}}
{{template "shared/issueicon" .}}
</div>
</div>
<div class="flex-item-main">
@ -19,7 +22,7 @@
{{template "repo/commit_statuses" dict "Status" (index $.CommitLastStatus .PullRequest.ID) "Statuses" (index $.CommitStatuses .PullRequest.ID)}}
{{end}}
{{end}}
<span class="labels-list tw-ml-1">
<span class="labels-list">
{{range .Labels}}
<a href="?q={{$.Keyword}}&type={{$.ViewType}}&state={{$.State}}&labels={{.ID}}{{if ne $.listType "milestone"}}&milestone={{$.MilestoneID}}{{end}}&assignee={{$.AssigneeID}}&poster={{$.PosterID}}{{if $.ShowArchivedLabels}}&archived=true{{end}}">{{ctx.RenderUtils.RenderLabel .}}</a>
{{end}}
@ -32,7 +35,7 @@
</div>
{{end}}
</div>
<div class="flex-item-body">
<div class="flex-item-body tw-mt-1">
<a class="index" href="{{if .Link}}{{.Link}}{{else}}{{$.Link}}/{{.Index}}{{end}}">
{{if eq $.listType "dashboard"}}
{{.Repo.FullName}}#{{.Index}}

View file

@ -34,6 +34,11 @@
/* z-index */
--z-index-modal: 1001; /* modal dialog, hard-coded from Fomantic modal.css */
--z-index-toast: 1002; /* should be larger than modal */
--font-size-label: 12px; /* font size of individual labels */
--gap-inline: 0.25rem; /* gap for inline texts and elements, for example: the spaces for sentence with labels, button text, etc */
--gap-block: 0.25rem; /* gap for element blocks, for example: spaces between buttons, menu image & title, header icon & title etc */
}
@media (min-width: 768px) and (max-width: 1200px) {
@ -1093,7 +1098,7 @@ table th[data-sortt-desc] .svg {
.flex-text-inline {
display: inline-flex;
align-items: center;
gap: .25rem;
gap: var(--gap-inline);
vertical-align: middle;
min-width: 0; /* make ellipsis work */
}
@ -1121,7 +1126,7 @@ table th[data-sortt-desc] .svg {
.flex-text-block {
display: flex;
align-items: center;
gap: .5rem;
gap: var(--gap-block);
min-width: 0;
}
@ -1136,7 +1141,7 @@ the "!important" is necessary to override Fomantic UI menu item styles, meanwhil
.ui.dropdown .menu.flex-items-menu > .item:not(.hidden, .filtered, .tw-hidden) {
display: flex !important;
align-items: center;
gap: .5rem;
gap: var(--gap-block);
min-width: 0;
}
.ui.dropdown .menu.flex-items-menu > .item img,

View file

@ -4,25 +4,19 @@
.ui.label {
display: inline-flex;
align-items: center;
gap: .25rem;
gap: var(--gap-inline);
min-width: 0;
vertical-align: middle;
line-height: 1;
max-width: 100%;
background: var(--color-label-bg);
color: var(--color-label-text);
padding: 0.3em 0.5em;
font-size: 0.85714286rem;
padding: 2px 6px;
font-size: var(--font-size-label);
font-weight: var(--font-weight-medium);
border: 0 solid transparent;
border-radius: 0.28571429rem;
border-radius: var(--border-radius);
white-space: nowrap;
}
.ui.label:first-child {
margin-left: 0;
}
.ui.label:last-child {
margin-right: 0;
overflow: hidden;
text-overflow: ellipsis;
}
a.ui.label {
@ -292,3 +286,58 @@ a.ui.ui.ui.basic.grey.label:hover {
.ui.large.label {
font-size: 1rem;
}
/* To let labels break up and wrap across multiple lines (issue title, comment event), use "display: contents here" to apply parent layout.
If the labels-list itself needs some layouts, use extra classes or "tw" helpers. */
.labels-list {
display: contents;
font-size: var(--font-size-label); /* it must match the label font size, otherwise the height mismatches */
}
.labels-list a {
max-width: 100%; /* for ellipsis */
}
.labels-list .ui.label {
min-height: 20px;
padding-top: 0;
padding-bottom: 0;
}
.with-labels-list-inline .labels-list .ui.label + .ui.label {
margin-left: var(--gap-inline);
}
.with-labels-list-inline .labels-list .ui.label {
line-height: var(--line-height-default);
}
/* Scoped labels with different colors on left and right */
.ui.label.scope-parent {
background: none !important;
padding: 0 !important;
gap: 0 !important;
}
.ui.label.scope-parent > .ui.label {
margin: 0 !important; /* scoped label's margin is handled by the parent */
}
.ui.label.scope-left {
border-bottom-right-radius: 0;
border-top-right-radius: 0;
}
.ui.label.scope-middle {
border-radius: 0;
}
.ui.label.scope-right {
border-bottom-left-radius: 0;
border-top-left-radius: 0;
}
.ui.label.archived-label {
filter: grayscale(0.5);
opacity: 0.5;
}

View file

@ -5,7 +5,6 @@
list-style-type: none;
margin: 1em 0;
padding: 0;
font-size: 1em;
}
.ui.list:first-child {

View file

@ -73,10 +73,21 @@
overflow: hidden;
}
.issue-content-right .ui.list {
margin: 0.5em 0;
max-width: 100%;
}
.issue-sidebar-combo > .ui.dropdown .item:not(.checked) .item-check-mark {
visibility: hidden;
}
.issue-content-right .ui.list.labels-list {
display: flex;
gap: var(--gap-inline);
flex-wrap: wrap;
}
@media (max-width: 767.98px) {
.issue-content-left,
.issue-content-right {
@ -1544,49 +1555,6 @@ td .commit-summary {
border-bottom-right-radius: 4px;
}
.labels-list {
display: inline-flex;
flex-wrap: wrap;
gap: 2.5px;
align-items: center;
}
.labels-list .label, .scope-parent > .label {
padding: 0 6px;
min-height: 20px;
line-height: 1.3; /* there is a `font-size: 1.25em` for inside emoji, so here the line-height needs to be larger slightly */
}
/* Scoped labels with different colors on left and right */
.ui.label.scope-parent {
background: none !important;
padding: 0 !important;
gap: 0 !important;
}
.archived-label {
filter: grayscale(0.5);
opacity: 0.5;
}
.ui.label.scope-left {
border-bottom-right-radius: 0;
border-top-right-radius: 0;
margin-right: 0;
}
.ui.label.scope-middle {
border-radius: 0;
margin-left: 0;
margin-right: 0;
}
.ui.label.scope-right {
border-bottom-left-radius: 0;
border-top-left-radius: 0;
margin-left: 0;
}
.repo-button-row {
margin: 8px 0;
display: flex;

View file

@ -29,13 +29,16 @@
display: flex;
width: 100%;
justify-content: space-between;
gap: 0.25em;
gap: 1em;
}
.issue-card-assignees {
.issue-card-bottom-part {
display: flex;
flex: 1;
align-items: center;
gap: 0.25em;
justify-content: end;
flex-wrap: wrap;
overflow: hidden;
max-width: fit-content;
max-height: fit-content;
}

View file

@ -4,41 +4,46 @@
margin: 0;
}
.issue-label-list .item {
.issue-label-list > .item {
border-bottom: 1px solid var(--color-secondary);
display: flex;
padding: 1em 0;
margin: 0;
}
.issue-label-list .item:first-child {
.issue-label-list > .item:first-child {
padding-top: 0;
}
.issue-label-list .item:last-child {
.issue-label-list > .item:last-child {
border-bottom: none;
padding-bottom: 0;
}
.issue-label-list .item .label-title {
.issue-label-list > .item .label-title {
width: 33%;
padding-right: 1em;
}
.issue-label-list .item .label-issues {
.issue-label-list > .item .label-issues {
width: 33%;
padding-right: 1em;
}
.issue-label-list .item .label-operation {
.issue-label-list > .item .label-operation {
width: 33%;
display: flex;
flex-wrap: wrap;
gap: 0.5em;
justify-content: end;
align-items: center;
}
.issue-label-list .item a {
.issue-label-list > .item .label-operation a {
font-size: 12px;
padding-right: 10px;
color: var(--color-text-light);
}
.issue-label-list .item.org-label {
.issue-label-list > .item.org-label {
opacity: 0.7;
}

View file

@ -33,14 +33,6 @@
color: var(--color-primary) !important;
}
.flex-item .flex-item-icon {
align-self: baseline; /* mainly used by the issue list, to align the leading icon with the title */
}
.flex-item .flex-item-icon + .flex-item-main {
align-self: baseline;
}
.flex-item .flex-item-trailing {
display: flex;
gap: 0.5rem;
@ -54,7 +46,9 @@
display: inline-flex;
flex-wrap: wrap;
align-items: center;
gap: .25rem;
/* labels are under effect of this gap here because they are display:contents. Ideally we should make wrapping
of labels work without display: contents and set this to a static value again. */
gap: var(--gap-inline);
max-width: 100%;
color: var(--color-text);
font-size: 16px;