Bug 566818 - New System theme styled form headers which looks bad
Summary: New System theme styled form headers which looks bad
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: IDE (show other bugs)
Version: 4.14   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-09 09:25 EDT by Lars Vogel CLA
Modified: 2020-09-10 09:10 EDT (History)
4 users (show)

See Also:


Attachments
Screenshot (140.27 KB, image/png)
2020-09-09 09:25 EDT, Lars Vogel CLA
no flags Details
Screenshot with Gerrit (71.11 KB, image/png)
2020-09-09 11:40 EDT, Lars Vogel CLA
no flags Details
Screenshot (80.81 KB, image/png)
2020-09-10 04:21 EDT, Lars Vogel CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2020-09-09 09:25:39 EDT
Created attachment 284095 [details]
Screenshot

See screenshot
Comment 1 Mickael Istria CLA 2020-09-09 09:41:10 EDT
I'm wondering what should the colors be for those form headers? Is there some system color that would fit for all OS/themes?
Comment 2 Eclipse Genie CLA 2020-09-09 10:42:17 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/169088
Comment 3 Mickael Istria CLA 2020-09-09 10:43:31 EDT
I've pushed https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/169088 for review.
Although I'm not certain about the design choice and the result, I believe it uses better system colors that fit better.
Please review whether this at least fix the readability issue; if it does, we can merge it as a 1st iteration and keep the issue open to figure out how this can be improved.
Comment 4 Mickael Istria CLA 2020-09-09 11:08:28 EDT
As I'm having a look at it, I'm wondering whether the TITLE colors wouldn't be a better suite for the tabs as well. Any opinion here?
Comment 6 Lars Vogel CLA 2020-09-09 11:40:29 EDT
Created attachment 284097 [details]
Screenshot with Gerrit

With the Gerrit, it looks even worse.
Comment 7 Mickael Istria CLA 2020-09-09 11:53:03 EDT
(In reply to Lars Vogel from comment #6)
> With the Gerrit, it looks even worse.

We can now read the text, so to me it's fixing an issue, not really worse. But indeed, the color of the text seem incorrect.
Comment 8 Lars Vogel CLA 2020-09-09 12:06:58 EDT
(In reply to Mickael Istria from comment #7)

> We can now read the text, so to me it's fixing an issue, not really worse.
> But indeed, the color of the text seem incorrect.

Ok, lets say still looks uglier.... ;-)
Comment 9 Mickael Istria CLA 2020-09-09 12:38:31 EDT
(In reply to Lars Vogel from comment #8)
> Ok, lets say still looks uglier.... ;-)

Can you please elaborate with more specific details? People have different tastes, and finding "visual" compromise that fit everyone usually work better when focusing on those details.
Comment 10 Lars Vogel CLA 2020-09-09 13:22:52 EDT
(In reply to Mickael Istria from comment #9)
> (In reply to Lars Vogel from comment #8)
> > Ok, lets say still looks uglier.... ;-)
> 
> Can you please elaborate with more specific details? People have different
> tastes, and finding "visual" compromise that fit everyone usually work
> better when focusing on those details.

Like you said: "But indeed, the color of the text seem incorrect."
Comment 11 Thomas Schindl CLA 2020-09-09 14:59:19 EDT
Instead of finding a color it would be better to introduce support for functions to compute the text color based on the background color - javafx css provides this and it super powerful
Comment 12 Andrew Obuchowicz CLA 2020-09-09 17:59:22 EDT
(In reply to Thomas Schindl from comment #11)
> Instead of finding a color it would be better to introduce support for
> functions to compute the text color based on the background color - javafx
> css provides this and it super powerful

I've been taking a similar approach for Spectrum Theme.
On the experimental branch of the project, I setup functions to use a light or dark font based on the perceived luminance (brightness) of the background color:
https://github.com/AObuchow/Eclipse-Spectrum-Theme/blob/experimental/com.aobuchow.themes.spectrum.preferences/src/com/aobuchow/themes/spectrum/preferences/ColorManager.java#L146

The actual perceived luminance formula used is here:
https://github.com/AObuchow/Eclipse-Spectrum-Theme/blob/experimental/com.aobuchow.themes.spectrum.preferences/src/com/aobuchow/themes/spectrum/preferences/ColorHSL.java#L101

When a theme change event occurs, the font colors are computed based on the users selected theme colors.


Maybe this could somehow be integrated into Eclipse's CSS, to allow font colors to be selected based on the background color that the font will e displayed on.
Comment 13 Mickael Istria CLA 2020-09-09 19:32:06 EDT
(In reply to Thomas Schindl from comment #11)
> Instead of finding a color...

in this case, we don't need to find a color. The colors are povided by the sytem and there I a good COLOR-TITLE-FOREGROUND to use. The CSS jut has some bug as it references the wrong color.
Comment 14 Eclipse Genie CLA 2020-09-10 04:12:18 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/169135
Comment 15 Mickael Istria CLA 2020-09-10 04:13:18 EDT
(In reply to Eclipse Genie from comment #14)
> New Gerrit change created:
> https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/169135

@Lars: can you please have a look at this new one with your Ubuntu theme?
Comment 16 Lars Vogel CLA 2020-09-10 04:21:10 EDT
Created attachment 284103 [details]
Screenshot

(In reply to Mickael Istria from comment #15)
> (In reply to Eclipse Genie from comment #14)
> > New Gerrit change created:
> > https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/169135
> 
> @Lars: can you please have a look at this new one with your Ubuntu theme?

Still looks way to colorful.
Comment 17 Mickael Istria CLA 2020-09-10 04:59:43 EDT
New patch set on the same gerrit review has some variation. Can you please check?

(In reply to Lars Vogel from comment #16)
> Still looks way to colorful.

Unfortunately, the system only properly define the COLOR_LIST* foreground/background color (usually black&white) for the text writing as the form background == list background, and some COLOR_TITLE* which are on main linux using some white text on orange or blue background.
If we assume that on all OS COLOR_LIST_BACKGROUND==COLOR_TITLE_FOREGROUND -like it's the case on GTK Adwaita light and Ubuntu themes-, we can just reverse the color and get a nice title bar with colors; it's what the latest patchset does. Otherwise, we'd better stick to black on white (or the other way round for dark themes).
But semantically, although it can be very colorful, I believe usung the COLOR_TITLE for title bars in forms is the most correct, and should ensure good readability on all OS.