Scoped label display improvements

[brechtvl]

• Use scoped label display and menu grouping also for non-exclusive labels with /. This means the Exclusive option no longer affects the label display, only the behavior when assigning labels.
• Fix scoped label left and right part breaking across lines.
• Remove slanted divider in scoped label display, make it straight. After using this for a while, this feels more visually noisy than helpful.
• Deduplicate template code for labels selection menu.
• Reduce contrast between scope and item to reduce probability of unreadable text on background.

⚠️ BREAKING ⚠️

Existing labels with a / character in the name are now displayed as a scoped label. These labels are not mutually exclusive by default. If the / character was used for another purpose, the label name can be changed to use another character or word.

Part of #22974

[brechtvl]

Before and after:

I tried to make labels not overflow like #23146, however I had trouble figuring out how to both make it break words within the left and right part, while still ensuring the left and right part don't break apart

[a1012112796]

how about add overflow: scroll style for lable list in sidebar?

[brechtvl]

It seems preferable to be consistent with #23146 if we can and use word-break? Though I have not succeeded in getting that to work.

I did make a change to inline-flex so that there is no overflow when using more reasonable length words.

Before	white-space: nowrap	display: inline-flex	display: inline-flex, word-break: break-all

[techknowlogick]

Thanks for the comparison screenshots. I do also prefer it with inline-flex.

LGTM ❤️

[codecov-commenter]

Codecov Report

Merging #23164 (8a402d0) into main (5c4075e) will increase coverage by 0.16%.
The diff coverage is 87.50%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main   #23164      +/-   ##
==========================================
+ Coverage   47.39%   47.56%   +0.16%     
==========================================
  Files        1143     1143              
  Lines      151093   151171      +78     
==========================================
+ Hits        71617    71909     +292     
+ Misses      71035    70757     -278     
- Partials     8441     8505      +64     

Impacted Files	Coverage Δ
modules/templates/helper.go	43.75% <50.00%> (+0.11%)	⬆️
models/issues/label.go	58.04% <100.00%> (+0.69%)	⬆️
modules/auth/password/hash/setting.go	68.18% <0.00%> (-31.82%)	⬇️
modules/indexer/stats/indexer.go	51.06% <0.00%> (-6.39%)	⬇️
modules/notification/mail/mail.go	29.41% <0.00%> (-2.95%)	⬇️
routers/api/packages/nuget/api_v3.go	94.49% <0.00%> (-2.76%)	⬇️
routers/install/routes.go	41.46% <0.00%> (-1.96%)	⬇️
modules/notification/ui/ui.go	60.34% <0.00%> (-1.73%)	⬇️
modules/queue/queue_channel.go	84.26% <0.00%> (-1.13%)	⬇️
models/activities/notification.go	61.33% <0.00%> (-1.12%)	⬇️
... and 18 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[wxiaoguang]

I tried to make labels not overflow like #23146, however I had trouble figuring out how to both make it break words within the left and right part, while still ensuring the left and right part don't break apart

Do you mean this? https://jsfiddle.net/L2o0kq3n/

ps: the inline may not work with other styles, it may need other tricks, just a guess.

[a1012112796]

in github it just cut the too long contents by css, but that's looks hard for scoped label. wonder how gitlab handle it...

[brechtvl]

I updated my comment above to show what it looks like with word-break: break-all. It avoids the overflow but also makes the regular cases without very long words worse. Breaking for example "Status" across multiple lines is not helping readability.

I think this PR is a net improvement over the current state which also does not stop label overflow, so maybe it's good enough to merge?

Do you mean this? https://jsfiddle.net/L2o0kq3n/

Not really, this doesn't look that good to me for cases like Status in my example. But it's a matter of taste also.

in github it just cut the too long contents by css, but that's looks hard for scoped label. wonder how gitlab handle it...

It cuts off the text on both sides. To me that behavior seems not useful either. If you see this you probably rename the label to something else to make it readable.

[lunny]

• Use scoped label display and menu grouping also for non-exclusive labels with /.

  • Fix scoped label left and right part breaking across lines.

  • Remove slanted divider in scoped label display, make it straight. After using this for a while, this feels more visually noisy than helpful.

  • Deduplicate template code for labels selection menu.

  • Reduce contrast between scope and item to reduce probability of unreadable text on background.

Part of #22974

Looks like the first item should be marked as a break change?

[wxiaoguang]

Looks like the first item should be marked as a break change?

Maybe not that breaking (how? I can not imagine ...), we can consider it as improvement?

---

Update: I see the breaking now. Some people just want to use literal labels like "N/A" without scope.

[brechtvl]

I updated the description to notify about the breaking change, assuming we are ok with this.

[lunny]

I updated the description to notify about the breaking change, assuming we are ok with this.

Thanks! Then maybe we also need to mention that the column of exclusive's meaning has been changed.

[brechtvl]

I updated the description to try to make that more explicit, but I'm not sure I understood exactly what you are asking.

[LamGC]

Can the repository choose to enable this feature? It would be better if users could choose whether to enable this feature in the repository.

[lunny]

I updated the description to try to make that more explicit, but I'm not sure I understood exactly what you are asking.

I mean maybe the description in locale-en_US.ini for the checkbox of exclusive on adding label UI should be changed because now whatever you check or not check exclusive, / will always be rendered as the Exclusive option no longer affects the label display.

[lunny]

Can the repository choose to enable this feature? It would be better if users could choose whether to enable this feature in the repository.

I don't think we are ready to add an option for repository, maybe depend on #22500

[yp05327]

I think we can add a setting to this in app.ini (global) and the repo settings.
The default value is an empty string and will have no changes.
If users want to use this new feature, they can select the character by themselves.

[BLumia]

Sorry I am strongly discouraged from doing this since using / for 'or' is not rare. See discussions in #22974 (comment) for the reason. I think it's okay to do so if we are using some rare delimiter like ::, but / is not rare, and also, we don't have the ability to escape the delimiter symbol...

[delvh]

We are getting severely offtopic for this PR

but:

I don't quite understand why anyone would need a global or even repo-specific configuration to enable or disable this feature.
You implicitly have to enable it yourself, as it is only activated for labels that explicitly set exclusive=true.
(Which is none of the existing labels by default).
The display improvements in this PR have nothing to do with the actual implementation of scoped labels.
Please, try out how the scoped labels are implemented at the moment before demanding being able to enable/disable this feature.
I'd say it is pretty intuitive and re-usable in how it is implemented at the moment.
Furthermore, you can choose for each label already if it should be enabled or disabled.
Any attempt to further enable/disable it is going to complicate the logic a lot, and I will likely strongly oppose adding this (in my eyes) unnecessary and unmaintainable overhead.

[BLumia]

We are getting severely offtopic for this PR

I'm sorry since the discussion in #22974 doesn't stop this from happening, so I left a comment to let people know the concern of the breaking change.

You implicitly have to enable it yourself, as it is only activated for labels that explicitly set exclusive=true.

The existing change is fine to me and I'm okay if we don't plan to switch to use another delimiter in database, but If the / character was used for another purpose, the label name can be changed to use another character or word. is a breaking change and I'm NOT okay with that.

If you are doing this, remove the exclusive field too since it will be very much useless or confuse the meaning of the exclusive option.

[lunny]

We are getting severely offtopic for this PR
but:

I don't quite understand why anyone would need a global or even repo-specific configuration to enable or disable this feature. You implicitly have to enable it yourself, as it is only activated for labels that explicitly set exclusive=true.

No. This PR will break this behaviour. Even exclusive=false, / will also be rendered.

(Which is none of the existing labels by default). The display improvements in this PR have nothing to do with the actual implementation of scoped labels. Please, try out how the scoped labels are implemented at the moment before demanding being able to enable/disable this feature. I'd say it is pretty intuitive and re-usable in how it is implemented at the moment. Furthermore, you can choose for each label already if it should be enabled or disabled. Any attempt to further enable/disable it is going to complicate the logic a lot, and I will likely strongly oppose adding this (in my eyes) unnecessary and unmaintainable overhead.

[wxiaoguang]

I can see a possible approach to avoid the breaking, introduce a label property like Groupable.

I think some people may want to use "N/A" , "mac/win" or "c/java" for labels without scope.

For label foo/bar:

• If Groupable=1, the new behavior: <div>foo</div><div>bar</div>
• If Groupable=0, the old behavior:<div>foo/bar</div>
• (it seems dirty when Groupable=0 and Exclusive=1. see the new thought below)

---

Or just change the Exclusive to a enum: Default/Groupable/Exclusive.

For label foo/bar:

• If Default, the old behavior:<div>foo/bar</div>
• If Groupable or Exclusive, the new behavior: <div>foo</div><div>bar</div>
• The Exclusive also controls other logic.

(Just a brief thought ....)

[delvh]

Hmm… Looks like we should postpone merging this PR as it is for now.
It looks like there is a lot to discuss before we can merge it.
Especially the main improvement of this PR, that all labels containing / get the visual prefix, no matter if they are scoped labels or not, seems to be highly controversial.
What we could do is extract a sub-PR that implements everything except for the breaking (and coincidentally controversial) part.
Then, we could drop everything else in this PR and make it only a v1.20 PR without backporting it to 1.19.
So far, I haven't seen any objection to any of the other improvements, which should be merged into 1.19 already.

[brechtvl]

The simpler PR for 1.19 is #23430, the code deduplication is in #23431.

[brechtvl]

I'll close this one and create a new PR with an enum for scoped labels later.