BUG: 382139
Previously these labels were white-on-light-blue with light themes; now they use the correct theme color property, and become black appropriately.
aacid | |
broulik | |
sander | |
rkflx |
Okular | |
KDE Applications |
BUG: 382139
Previously these labels were white-on-light-blue with light themes; now they use the correct theme color property, and become black appropriately.
Tested on KDE Neon (dev unstable). Verified that with the default Breeze color scheme, tab text is more readable when hovered over and then active but not in focus. See before and after:
Also tested these changes with all standard color schemes as well as the Oxygen theme to make sure that there were no visual regressions.
Lint Skipped |
Unit Tests Skipped |
I don't think that's the oxygen style, that may be the oxygen color theme.
okular -style oxygen
is what i mean
Great, one item less on my list of things to fix in Okular :)
@ngraham Just a suggestion for future Diffs: Have a look at git's 50/72 rule (…of thumb) and the reasoning behind it, as the title of your commit message is quite long, really ;-)
Also small coding style nit: superfluous parens could be removed, but not really important
Thanks. Title is still unchanged, but this was more of a hint for future contributions, anyway.
Given you are now listed as "KDE developer", I assume you can commit by yourself? I guess this should go into the bugfix branch (Applications/17.08), then merge to master.
Thanks Henrik! Yes, I have a developer account and should be able to commit now. I haven't had the time to actually set that up yet though. I'll look into it late tonight or tomorrow.
I managed to get arc set up. Am I basically going to do this?
One potential issue: arc land --onto Applications/17.08 --preview shows a large number of extraneous commits that would be landed onto that branch, not just my own.
arc will squash and push all commits from the current branch which are not yet in the target branch, in your case all commits exclusive to master in addition to your patch. As you noticed correctly, that's not what you'll want. I'd probably prefer to move such a patch to Applications/17.08 locally via git first, to check everything worked and still compiles:
Now for the merge (see also here):