Skip to content

Commit 6221a6f

Browse files
brechtvlyardenshohamlafriks
authored
Scoped labels (#22585)
Add a new "exclusive" option per label. This makes it so that when the label is named `scope/name`, no other label with the same `scope/` prefix can be set on an issue. The scope is determined by the last occurence of `/`, so for example `scope/alpha/name` and `scope/beta/name` are considered to be in different scopes and can coexist. Exclusive scopes are not enforced by any database rules, however they are enforced when editing labels at the models level, automatically removing any existing labels in the same scope when either attaching a new label or replacing all labels. In menus use a circle instead of checkbox to indicate they function as radio buttons per scope. Issue filtering by label ensures that only a single scoped label is selected at a time. Clicking with alt key can be used to remove a scoped label, both when editing individual issues and batch editing. Label rendering refactor for consistency and code simplification: * Labels now consistently have the same shape, emojis and tooltips everywhere. This includes the label list and label assignment menus. * In label list, show description below label same as label menus. * Don't use exactly black/white text colors to look a bit nicer. * Simplify text color computation. There is no point computing luminance in linear color space, as this is a perceptual problem and sRGB is closer to perceptually linear. * Increase height of label assignment menus to show more labels. Showing only 3-4 labels at a time leads to a lot of scrolling. * Render all labels with a new RenderLabel template helper function. Label creation and editing in multiline modal menu: * Change label creation to open a modal menu like label editing. * Change menu layout to place name, description and colors on separate lines. * Don't color cancel button red in label editing modal menu. * Align text to the left in model menu for better readability and consistent with settings layout elsewhere. Custom exclusive scoped label rendering: * Display scoped label prefix and suffix with slightly darker and lighter background color respectively, and a slanted edge between them similar to the `/` symbol. * In menus exclusive labels are grouped with a divider line. --------- Co-authored-by: Yarden Shoham <hrsi88@gmail.com> Co-authored-by: Lauris BH <lauris@nix.lv>
1 parent feed1ff commit 6221a6f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+709
-242
lines changed

Diff for: models/fixtures/issue.yml

+17
Original file line numberDiff line numberDiff line change
@@ -287,3 +287,20 @@
287287
created_unix: 1602935696
288288
updated_unix: 1602935696
289289
is_locked: false
290+
291+
-
292+
id: 18
293+
repo_id: 55
294+
index: 1
295+
poster_id: 2
296+
original_author_id: 0
297+
name: issue for scoped labels
298+
content: content
299+
milestone_id: 0
300+
priority: 0
301+
is_closed: false
302+
is_pull: false
303+
num_comments: 0
304+
created_unix: 946684830
305+
updated_unix: 978307200
306+
is_locked: false

Diff for: models/fixtures/label.yml

+45
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
org_id: 0
55
name: label1
66
color: '#abcdef'
7+
exclusive: false
78
num_issues: 2
89
num_closed_issues: 0
910

@@ -13,6 +14,7 @@
1314
org_id: 0
1415
name: label2
1516
color: '#000000'
17+
exclusive: false
1618
num_issues: 1
1719
num_closed_issues: 1
1820

@@ -22,6 +24,7 @@
2224
org_id: 3
2325
name: orglabel3
2426
color: '#abcdef'
27+
exclusive: false
2528
num_issues: 0
2629
num_closed_issues: 0
2730

@@ -31,6 +34,7 @@
3134
org_id: 3
3235
name: orglabel4
3336
color: '#000000'
37+
exclusive: false
3438
num_issues: 1
3539
num_closed_issues: 0
3640

@@ -40,5 +44,46 @@
4044
org_id: 0
4145
name: pull-test-label
4246
color: '#000000'
47+
exclusive: false
48+
num_issues: 0
49+
num_closed_issues: 0
50+
51+
-
52+
id: 6
53+
repo_id: 55
54+
org_id: 0
55+
name: unscoped_label
56+
color: '#000000'
57+
exclusive: false
58+
num_issues: 0
59+
num_closed_issues: 0
60+
61+
-
62+
id: 7
63+
repo_id: 55
64+
org_id: 0
65+
name: scope/label1
66+
color: '#000000'
67+
exclusive: true
68+
num_issues: 0
69+
num_closed_issues: 0
70+
71+
-
72+
id: 8
73+
repo_id: 55
74+
org_id: 0
75+
name: scope/label2
76+
color: '#000000'
77+
exclusive: true
78+
num_issues: 0
79+
num_closed_issues: 0
80+
81+
-
82+
id: 9
83+
repo_id: 55
84+
org_id: 0
85+
name: scope/subscope/label2
86+
color: '#000000'
87+
exclusive: true
4388
num_issues: 0
4489
num_closed_issues: 0

Diff for: models/fixtures/repository.yml

+12
Original file line numberDiff line numberDiff line change
@@ -1622,3 +1622,15 @@
16221622
is_archived: false
16231623
is_private: true
16241624
status: 0
1625+
1626+
-
1627+
id: 55
1628+
owner_id: 2
1629+
owner_name: user2
1630+
lower_name: scoped_label
1631+
name: scoped_label
1632+
is_empty: false
1633+
is_archived: false
1634+
is_private: true
1635+
num_issues: 1
1636+
status: 0

Diff for: models/fixtures/user.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
num_followers: 2
6767
num_following: 1
6868
num_stars: 2
69-
num_repos: 10
69+
num_repos: 11
7070
num_teams: 0
7171
num_members: 0
7272
visibility: 0

Diff for: models/issues/issue.go

+27
Original file line numberDiff line numberDiff line change
@@ -538,6 +538,31 @@ func (ts labelSorter) Swap(i, j int) {
538538
[]*Label(ts)[i], []*Label(ts)[j] = []*Label(ts)[j], []*Label(ts)[i]
539539
}
540540

541+
// Ensure only one label of a given scope exists, with labels at the end of the
542+
// array getting preference over earlier ones.
543+
func RemoveDuplicateExclusiveLabels(labels []*Label) []*Label {
544+
validLabels := make([]*Label, 0, len(labels))
545+
546+
for i, label := range labels {
547+
scope := label.ExclusiveScope()
548+
if scope != "" {
549+
foundOther := false
550+
for _, otherLabel := range labels[i+1:] {
551+
if otherLabel.ExclusiveScope() == scope {
552+
foundOther = true
553+
break
554+
}
555+
}
556+
if foundOther {
557+
continue
558+
}
559+
}
560+
validLabels = append(validLabels, label)
561+
}
562+
563+
return validLabels
564+
}
565+
541566
// ReplaceIssueLabels removes all current labels and add new labels to the issue.
542567
// Triggers appropriate WebHooks, if any.
543568
func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (err error) {
@@ -555,6 +580,8 @@ func ReplaceIssueLabels(issue *Issue, labels []*Label, doer *user_model.User) (e
555580
return err
556581
}
557582

583+
labels = RemoveDuplicateExclusiveLabels(labels)
584+
558585
sort.Sort(labelSorter(labels))
559586
sort.Sort(labelSorter(issue.Labels))
560587

Diff for: models/issues/issue_test.go

+12-7
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
func TestIssue_ReplaceLabels(t *testing.T) {
2626
assert.NoError(t, unittest.PrepareTestDatabase())
2727

28-
testSuccess := func(issueID int64, labelIDs []int64) {
28+
testSuccess := func(issueID int64, labelIDs, expectedLabelIDs []int64) {
2929
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: issueID})
3030
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID})
3131
doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
@@ -35,15 +35,20 @@ func TestIssue_ReplaceLabels(t *testing.T) {
3535
labels[i] = unittest.AssertExistsAndLoadBean(t, &issues_model.Label{ID: labelID, RepoID: repo.ID})
3636
}
3737
assert.NoError(t, issues_model.ReplaceIssueLabels(issue, labels, doer))
38-
unittest.AssertCount(t, &issues_model.IssueLabel{IssueID: issueID}, len(labelIDs))
39-
for _, labelID := range labelIDs {
38+
unittest.AssertCount(t, &issues_model.IssueLabel{IssueID: issueID}, len(expectedLabelIDs))
39+
for _, labelID := range expectedLabelIDs {
4040
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: labelID})
4141
}
4242
}
4343

44-
testSuccess(1, []int64{2})
45-
testSuccess(1, []int64{1, 2})
46-
testSuccess(1, []int64{})
44+
testSuccess(1, []int64{2}, []int64{2})
45+
testSuccess(1, []int64{1, 2}, []int64{1, 2})
46+
testSuccess(1, []int64{}, []int64{})
47+
48+
// mutually exclusive scoped labels 7 and 8
49+
testSuccess(18, []int64{6, 7}, []int64{6, 7})
50+
testSuccess(18, []int64{7, 8}, []int64{8})
51+
testSuccess(18, []int64{6, 8, 7}, []int64{6, 7})
4752
}
4853

4954
func Test_GetIssueIDsByRepoID(t *testing.T) {
@@ -523,5 +528,5 @@ func TestCountIssues(t *testing.T) {
523528
assert.NoError(t, unittest.PrepareTestDatabase())
524529
count, err := issues_model.CountIssues(db.DefaultContext, &issues_model.IssuesOptions{})
525530
assert.NoError(t, err)
526-
assert.EqualValues(t, 17, count)
531+
assert.EqualValues(t, 18, count)
527532
}

Diff for: models/issues/label.go

+65-41
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ package issues
77
import (
88
"context"
99
"fmt"
10-
"html/template"
11-
"math"
1210
"regexp"
1311
"strconv"
1412
"strings"
@@ -89,6 +87,7 @@ type Label struct {
8987
RepoID int64 `xorm:"INDEX"`
9088
OrgID int64 `xorm:"INDEX"`
9189
Name string
90+
Exclusive bool
9291
Description string
9392
Color string `xorm:"VARCHAR(7)"`
9493
NumIssues int
@@ -128,18 +127,22 @@ func (label *Label) CalOpenOrgIssues(ctx context.Context, repoID, labelID int64)
128127
}
129128

130129
// LoadSelectedLabelsAfterClick calculates the set of selected labels when a label is clicked
131-
func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64) {
130+
func (label *Label) LoadSelectedLabelsAfterClick(currentSelectedLabels []int64, currentSelectedExclusiveScopes []string) {
132131
var labelQuerySlice []string
133132
labelSelected := false
134133
labelID := strconv.FormatInt(label.ID, 10)
135-
for _, s := range currentSelectedLabels {
134+
labelScope := label.ExclusiveScope()
135+
for i, s := range currentSelectedLabels {
136136
if s == label.ID {
137137
labelSelected = true
138138
} else if -s == label.ID {
139139
labelSelected = true
140140
label.IsExcluded = true
141141
} else if s != 0 {
142-
labelQuerySlice = append(labelQuerySlice, strconv.FormatInt(s, 10))
142+
// Exclude other labels in the same scope from selection
143+
if s < 0 || labelScope == "" || labelScope != currentSelectedExclusiveScopes[i] {
144+
labelQuerySlice = append(labelQuerySlice, strconv.FormatInt(s, 10))
145+
}
143146
}
144147
}
145148
if !labelSelected {
@@ -159,49 +162,43 @@ func (label *Label) BelongsToRepo() bool {
159162
return label.RepoID > 0
160163
}
161164

162-
// SrgbToLinear converts a component of an sRGB color to its linear intensity
163-
// See: https://en.wikipedia.org/wiki/SRGB#The_reverse_transformation_(sRGB_to_CIE_XYZ)
164-
func SrgbToLinear(color uint8) float64 {
165-
flt := float64(color) / 255
166-
if flt <= 0.04045 {
167-
return flt / 12.92
165+
// Get color as RGB values in 0..255 range
166+
func (label *Label) ColorRGB() (float64, float64, float64, error) {
167+
color, err := strconv.ParseUint(label.Color[1:], 16, 64)
168+
if err != nil {
169+
return 0, 0, 0, err
168170
}
169-
return math.Pow((flt+0.055)/1.055, 2.4)
170-
}
171-
172-
// Luminance returns the luminance of an sRGB color
173-
func Luminance(color uint32) float64 {
174-
r := SrgbToLinear(uint8(0xFF & (color >> 16)))
175-
g := SrgbToLinear(uint8(0xFF & (color >> 8)))
176-
b := SrgbToLinear(uint8(0xFF & color))
177171

178-
// luminance ratios for sRGB
179-
return 0.2126*r + 0.7152*g + 0.0722*b
172+
r := float64(uint8(0xFF & (uint32(color) >> 16)))
173+
g := float64(uint8(0xFF & (uint32(color) >> 8)))
174+
b := float64(uint8(0xFF & uint32(color)))
175+
return r, g, b, nil
180176
}
181177

182-
// LuminanceThreshold is the luminance at which white and black appear to have the same contrast
183-
// i.e. x such that 1.05 / (x + 0.05) = (x + 0.05) / 0.05
184-
// i.e. math.Sqrt(1.05*0.05) - 0.05
185-
const LuminanceThreshold float64 = 0.179
186-
187-
// ForegroundColor calculates the text color for labels based
188-
// on their background color.
189-
func (label *Label) ForegroundColor() template.CSS {
178+
// Determine if label text should be light or dark to be readable on background color
179+
func (label *Label) UseLightTextColor() bool {
190180
if strings.HasPrefix(label.Color, "#") {
191-
if color, err := strconv.ParseUint(label.Color[1:], 16, 64); err == nil {
192-
// NOTE: see web_src/js/components/ContextPopup.vue for similar implementation
193-
luminance := Luminance(uint32(color))
194-
195-
// prefer white or black based upon contrast
196-
if luminance < LuminanceThreshold {
197-
return template.CSS("#fff")
198-
}
199-
return template.CSS("#000")
181+
if r, g, b, err := label.ColorRGB(); err == nil {
182+
// Perceived brightness from: https://www.w3.org/TR/AERT/#color-contrast
183+
// In the future WCAG 3 APCA may be a better solution
184+
brightness := (0.299*r + 0.587*g + 0.114*b) / 255
185+
return brightness < 0.35
200186
}
201187
}
202188

203-
// default to black
204-
return template.CSS("#000")
189+
return false
190+
}
191+
192+
// Return scope substring of label name, or empty string if none exists
193+
func (label *Label) ExclusiveScope() string {
194+
if !label.Exclusive {
195+
return ""
196+
}
197+
lastIndex := strings.LastIndex(label.Name, "/")
198+
if lastIndex == -1 || lastIndex == 0 || lastIndex == len(label.Name)-1 {
199+
return ""
200+
}
201+
return label.Name[:lastIndex]
205202
}
206203

207204
// NewLabel creates a new label
@@ -253,7 +250,7 @@ func UpdateLabel(l *Label) error {
253250
if !LabelColorPattern.MatchString(l.Color) {
254251
return fmt.Errorf("bad color code: %s", l.Color)
255252
}
256-
return updateLabelCols(db.DefaultContext, l, "name", "description", "color")
253+
return updateLabelCols(db.DefaultContext, l, "name", "description", "color", "exclusive")
257254
}
258255

259256
// DeleteLabel delete a label
@@ -620,6 +617,29 @@ func newIssueLabel(ctx context.Context, issue *Issue, label *Label, doer *user_m
620617
return updateLabelCols(ctx, label, "num_issues", "num_closed_issue")
621618
}
622619

620+
// Remove all issue labels in the given exclusive scope
621+
func RemoveDuplicateExclusiveIssueLabels(ctx context.Context, issue *Issue, label *Label, doer *user_model.User) (err error) {
622+
scope := label.ExclusiveScope()
623+
if scope == "" {
624+
return nil
625+
}
626+
627+
var toRemove []*Label
628+
for _, issueLabel := range issue.Labels {
629+
if label.ID != issueLabel.ID && issueLabel.ExclusiveScope() == scope {
630+
toRemove = append(toRemove, issueLabel)
631+
}
632+
}
633+
634+
for _, issueLabel := range toRemove {
635+
if err = deleteIssueLabel(ctx, issue, issueLabel, doer); err != nil {
636+
return err
637+
}
638+
}
639+
640+
return nil
641+
}
642+
623643
// NewIssueLabel creates a new issue-label relation.
624644
func NewIssueLabel(issue *Issue, label *Label, doer *user_model.User) (err error) {
625645
if HasIssueLabel(db.DefaultContext, issue.ID, label.ID) {
@@ -641,6 +661,10 @@ func NewIssueLabel(issue *Issue, label *Label, doer *user_model.User) (err error
641661
return nil
642662
}
643663

664+
if err = RemoveDuplicateExclusiveIssueLabels(ctx, issue, label, doer); err != nil {
665+
return nil
666+
}
667+
644668
if err = newIssueLabel(ctx, issue, label, doer); err != nil {
645669
return err
646670
}

0 commit comments

Comments
 (0)