Skip to content

Compute proper foreground color for labels#16729

Merged
jolheiser merged 4 commits into
go-gitea:mainfrom
clarfonthey:luminance
Aug 25, 2021
Merged

Compute proper foreground color for labels#16729
jolheiser merged 4 commits into
go-gitea:mainfrom
clarfonthey:luminance

Conversation

@clarfonthey

@clarfonthey clarfonthey commented Aug 19, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

This properly ensures that the contrast ratio is maximised for any given color choice.

The comments explain the exact calculations done (you can check that I transcribed it correctly), but to fully clarify for review:

  1. Human eyes are better at detecting contrast than absolute differences in brightness. So, it's easier to see the difference between 0% and 10% brightness, than 90% and 100% brightness, even though it's the same absolute difference.
  2. As a result, sRGB scales the colours so that the 0-255 range for each component has more dark shades than bright shades.
  3. If you want to check the contrast between two colours, you need to undo this scaling.

@clarfonthey clarfonthey force-pushed the luminance branch 5 times, most recently from 6221115 to 2456418 Compare August 19, 2021 04:28
@codecov-commenter

This comment has been minimized.

Sign in to view
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 19, 2021
@silverwind

silverwind commented Aug 19, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Note there is a similar function in JS that should be made to match the backend implementation:

gitea/web_src/js/components/ContextPopup.vue

Lines 75 to 86 in c9bca8c

labels() {
return this.issue.labels.map((label) => {
const red = parseInt(label.color.substring(0, 2), 16);
const green = parseInt(label.color.substring(2, 4), 16);
const blue = parseInt(label.color.substring(4, 6), 16);
let color = '#ffffff';
if ((red * 0.299 + green * 0.587 + blue * 0.114) > 125) {
color = '#000000';
}
return {name: label.name, color: `#${label.color}`, textColor: color};
});
}

@clarfonthey

clarfonthey commented Aug 19, 2021

Copy link
Copy Markdown
Contributor Author

Good catch! I'll fix that this evening

@clarfonthey clarfonthey mentioned this pull request Aug 20, 2021
Open
@clarfonthey

clarfonthey commented Aug 20, 2021

Copy link
Copy Markdown
Contributor Author

Fixed!

@clarfonthey clarfonthey changed the title Actually compute proper foreground color for labels Compute proper foreground color for labels Aug 20, 2021
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 21, 2021
@jolheiser jolheiser added the topic/ui Change the appearance of the Gitea UI label Aug 24, 2021
@jolheiser jolheiser added this to the 1.16.0 milestone Aug 24, 2021
@jolheiser

jolheiser commented Aug 24, 2021

Copy link
Copy Markdown
Member

I think this all looks great, would you be able to attach a couple example images?

@clarfonthey

clarfonthey commented Aug 24, 2021

Copy link
Copy Markdown
Contributor Author

Sure thing! I'll take some tonight with some sample colours.

jolheiser
jolheiser approved these changes Aug 24, 2021
View reviewed changes

@jolheiser jolheiser left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Disregard my other comment, for some reason I thought this was changing colors, rather it's just a different formula for choosing white/black text.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 24, 2021
@clarfonthey

clarfonthey commented Aug 24, 2021

Copy link
Copy Markdown
Contributor Author

I'll still take some tonight anyway, since it might be helpful for the changelog

@jolheiser jolheiser merged commit 29b971b into go-gitea:main Aug 25, 2021
@clarfonthey clarfonthey deleted the luminance branch August 26, 2021 17:05
@go-gitea go-gitea locked and limited conversation to collaborators Oct 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@zeripath zeripath zeripath approved these changes

@jolheiser jolheiser jolheiser approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI

Projects

None yet

Milestone

1.16.0

Development

Successfully merging this pull request may close these issues.

6 participants

@clarfonthey @codecov-commenter @silverwind @jolheiser @zeripath @GiteaBot