Skip to content

Fix arc theme label backgrounds #13267

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 22, 2020
Merged

Fix arc theme label backgrounds #13267

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ca85c9
Fix arc theme label backgrounds
ivanvc Oct 22, 2020
ee4d020
Remove unnecesary selector
ivanvc Oct 22, 2020
081cde4
Merge branch 'master' into fix-arc-theme-background-labels
zeripath Oct 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions web_src/less/themes/theme-arc-green.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,12 @@ footer {
border-color: #8a2121 !important;
}

.ui.yellow.label,
.ui.yellow.labels .label {
border-color: #664d02 !important;
background-color: #936e00 !important;
}

.ui.menu {
background: #404552;
border: 1px solid #353945;
Expand Down Expand Up @@ -614,10 +620,11 @@ footer {
}

.ui.label,
.ui.label.basic {
.ui.label.basic,
.theme-arc-green .ui.label.basic {
Copy link
Member

@silverwind silverwind Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think this selector may be unnecessary. You generally always want to match the selector used in the light theme (so it overrules by CSS rule order) and this file is only included if theme is set to arc-green.

Copy link
Contributor Author

@ivanvc ivanvc Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

You're right it's not necessary, I was trying to avoid the precending rule from Fomantic UI, but all of those .ui.ui.ui make it more specific and override even having the .theme-arc-green selector:

Screenshot from 2020-10-22 11-07-04

Result:
image

Which forces the white background, and it makes it hard to read bright colors.

Would a good be to specify each of the possible background colors for the Fomantic UI base colors?
Screenshot from 2020-10-22 11-11-45 (reference: https://fomantic-ui.com/elements/label.html, removing the inverted class from the labels)

Or would that be out of the scope of the theme, as seems like those colors are not being used at all?

Copy link
Member

@silverwind silverwind Oct 22, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Those white backgrounds are bugs and should be overridden with the exact same selector fomantic uses, even if it has all those .ui. Feel free to do that in another PR.

color: #dbdbdb;
border-color: rgb(152, 152, 152);
background-color: #2a2e39 !important;
background-color: #2a2e39;
}

.issue.list > .item .title {
Expand Down