Currently labels on tabs in firefox that are not selected are hard to read (black text on dark background). This fixes it.
BUG: 407639
No Linters Available |
No Unit Test Coverage |
Buildable 13932 | |
Build 13950: arc lint + arc unit |
I'm not a Breeze GTK maintainer or developer, but perhaps you need to create separate folder for application specific styles, e.g.
src/XXX/apps
and put Firefox styles in a corresponding file, e.g.
src/XXX/apps/_firefox.scss
src/gtk320/widgets/_menus.scss | ||
---|---|---|
258 ↗ | (On Diff #61728) | It'd be better not to hard code the color. |
src/gtk320/apps/_firefox.scss | ||
---|---|---|
5 | I see that you are hardcoding a color here. Have you tested the change with Breeze Dark? |
I seem to have some problems building this, I guess I need to change something in build files. Could you help me find it, I am new at this?
There is currently no maintainer of breeze-gtk
How did you get it working in the first place? It builds fine for me, but it seems to do nothing, even when I add @import "apps/firefox"; to src/gtk320/gtk.scss
... it seems to do nothing ...
The problem was that I tried to use a color name instead of a hex code and put a wrong name there (still don't know the correct one).
This causes a problem with Breeze and Breeze Dark when the titlebar is enabled.
Breeze:
Breeze:
Yes, I see it is different, but isn't it better now (after the patch)? https://imgur.com/a/aauVgPp
Whether or not it's better is not relevant for this patch, which right now is only about fixing the text color. It's best not to mix multiple changes like this, unless you want to expand the patch in scope, in which case it should be re-titled accordingly.
Still, I would recommend doing one change per patch, because it makes each one more likely to get in compared to patches with multiple changes (especially when any one of those individual changes may be controversial or require discussion).
I would probably replace
color: #eff0f1; -> color: t_color(text);
background-color: #475057; -> background-color: t_color(background);
in _firefox.scss to not hardcode the colors
We should not style server-side decorated Firefox windows. They are fine.
The problem you're trying to solve only affects client-side decorated Firefox windows
:backdrop selector currently doesn't have any effect, see https://bugzilla.mozilla.org/show_bug.cgi?id=1562507.
If I did re-title and change the description, would this have a better chance of passing? The chage is so small (number of lines) that It probably wouldn't make sense to split it. If not, then I am stuck and I have clearly bitten more than I can chew.
Yes, but then you would still have to convince people that the other included color change is good as well.
The chage is so small (number of lines) that It probably wouldn't make sense to split it.
It's not about number of lines; it's about how many distinct changes it introduces. Imagine a three-line patch that introduces three changes, where one is a bugfix and two of which are potentially controversial visual or behavioral changes. It's a small patch, but it would benefit from being split into three so the uncontroversial bugfix can be committed ASAP, without getting derailed by a discussion of the other issues.
Does that make sense? If the problem here is that you don't know how to separate your bugfix from the visual change, I would encourage you to big into that a bit!
The latest version of this patch is better because it works well for people using the Breeze GTK theme with the Breeze colorscheme and it doesn't make the Breeze Dark GTK theme worse when combined with the Breeze Dark colorscheme. However, it does cause Breeze GTK to look odd when used with the Breeze Dark colorscheme:
Normally, I wouldn't consider this too much of a problem since people usually want to use the same theme system wide, but sometimes apps that use GTK themes don't play well with dark GTK themes. It is realistic to assume some people who normally use Breeze Dark will use the Breeze GTK theme via the system settings rather than an environment variable for the few problematic apps (GTK_THEME=Breeze app-name-here). Examples of apps that don't play well with dark GTK themes are Eclipse, Intellij IDEA and Firefox (before version 68).