Bug 565951 - Let CTabFolder view content use default background color instead of tab color
Summary: Let CTabFolder view content use default background color instead of tab color
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.16   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.17 M3   Edit
Assignee: Mickael Istria CLA
QA Contact:
URL:
Whiteboard:
Keywords: noteworthy
Depends on:
Blocks:
 
Reported: 2020-08-10 08:09 EDT by Mickael Istria CLA
Modified: 2020-09-01 05:19 EDT (History)
5 users (show)

See Also:


Attachments
Screenshot (23.07 KB, image/png)
2020-08-11 03:27 EDT, Lars Vogel CLA
no flags Details
before.gif (69.05 KB, image/gif)
2020-08-17 10:28 EDT, Amit Mendapara CLA
no flags Details
after.gif (74.97 KB, image/gif)
2020-08-17 10:28 EDT, Amit Mendapara CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mickael Istria CLA 2020-08-10 08:09:37 EDT
Currently, the CTabRendering forces that the background of the Tab element *content* is derived from the tab color. This can be (and often is) undesired for themes that want to highlight the tab with some color without willing to change the "lines" color.
We should add to theming a way to instruct whether to propagate tab colors to tab contents or not.
Comment 1 Eclipse Genie CLA 2020-08-10 08:25:11 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167476
Comment 2 Rolf Theunissen CLA 2020-08-10 09:35:52 EDT
I wonder how this relates to Bug 546987, it might be fixed by these changes or it might affect this change too.
Comment 3 Mickael Istria CLA 2020-08-10 09:40:17 EDT
(In reply to Rolf Theunissen from comment #2)
> I wonder how this relates to Bug 546987, it might be fixed by these changes
> or it might affect this change too.

I think the proposed change would basically make bug 546987 not surface, unless the ctabfolder is explicitly requested to inherit the tab color to children composites (eg toolbar)
Comment 4 Andrew Obuchowicz CLA 2020-08-10 11:07:41 EDT
+1 for fixing this, it's caused me some troubling workarounds downstream: https://github.com/AObuchow/Eclipse-Spectrum-Theme/issues/36
Comment 6 Lars Vogel CLA 2020-08-11 03:27:09 EDT
Created attachment 283828 [details]
Screenshot

In its current state, the view toolbar looks like is belongs to the non-active part see screenshot.

I know you are planning more changes in this area but this change alone does IMHO not look good.
Comment 7 Lars Vogel CLA 2020-08-11 03:41:06 EDT
Please add to N&N
Comment 8 Amit Mendapara CLA 2020-08-15 09:00:58 EDT
These changes has broken light theme on Linux.
Comment 9 Amit Mendapara CLA 2020-08-15 09:03:44 EDT
(In reply to Amit Mendapara from comment #8)
> These changes has broken light theme on Linux.

Sorry, not these changes. The light theme is broken though, I will check further.
Comment 10 Amit Mendapara CLA 2020-08-15 09:09:29 EDT
I can confirm. These changes has partly broken the light theme on Linux.
Comment 11 Rolf Theunissen CLA 2020-08-15 09:24:52 EDT
(In reply to Amit Mendapara from comment #10)
> I can confirm. These changes has partly broken the light theme on Linux.

What is broken? What is the problem you observe?
Comment 12 Amit Mendapara CLA 2020-08-15 11:36:31 EDT
As Lars mentioned in a previous comment, there is an issue with toolbar background and unselected tab text color is became unreadable.
Comment 13 Mickael Istria CLA 2020-08-16 17:47:24 EDT
I'm going to change the outline to a white line, so it will be a bit clearer that the content of the view belongs to the active tab.
Comment 14 Eclipse Genie CLA 2020-08-16 17:51:40 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167776
Comment 16 Amit Mendapara CLA 2020-08-17 01:56:45 EDT
I am still seeing the issues. Also, the last commit introduced another issue when you hover on an unselected tab.

I just tried reverting these two commits:

3a36645ba15be9300cec478904018137f51eaccb
b35d6ef53e51908f32af678c3c1a0992221f73ff

and everything was fine. I think these changes should be reverted and should be reworked in 4.18.
Comment 17 Mickael Istria CLA 2020-08-17 04:02:02 EDT
All that said, I'll probably change the current theme so that it takes advantage of the new "swt-draw-custom-tab-content-background: true;" to just restore the old behavior.
For the other patches I've linked, I'll basically start a new theme anyway.
Comment 18 Eclipse Genie CLA 2020-08-17 06:07:07 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167809
Comment 19 Mickael Istria CLA 2020-08-17 08:24:42 EDT
I've basically restored the current light theme to its current form, taking advantage of the new setting to force propagation of the tab color as content background and restoring the workaround for toolbar color in the CSS.
Further work will happen in a separate theme.
Comment 21 Rolf Theunissen CLA 2020-08-17 09:42:56 EDT
(In reply to Mickael Istria from comment #19)
> I've basically restored the current light theme to its current form, taking
> advantage of the new setting to force propagation of the tab color as
> content background and restoring the workaround for toolbar color in the CSS.
> Further work will happen in a separate theme.

Can you also run a test with '-cssTheme none' as program argument? To check that the current state is desirable.
Comment 22 Amit Mendapara CLA 2020-08-17 09:48:33 EDT
(In reply to Mickael Istria from comment #19)
> I've basically restored the current light theme to its current form, taking
> advantage of the new setting to force propagation of the tab color as
> content background and restoring the workaround for toolbar color in the CSS.
> Further work will happen in a separate theme.

Just tested the latest changes. Now it's working fine. However, gerrit https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167776 should be reverted.
Comment 23 Mickael Istria CLA 2020-08-17 09:51:31 EDT
(In reply to Amit Mendapara from comment #22)
> Just tested the latest changes. Now it's working fine. However, gerrit
> https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167776 should be
> reverted.

It won't be fully reverted as it's useful for some other thmes.
However, if you identify some specific bugs in this patch, those can be fixed.
Comment 24 Amit Mendapara CLA 2020-08-17 09:52:42 EDT
(In reply to Mickael Istria from comment #23)
> (In reply to Amit Mendapara from comment #22)
> > Just tested the latest changes. Now it's working fine. However, gerrit
> > https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167776 should be
> > reverted.
> 
> It won't be fully reverted as it's useful for some other thmes.
> However, if you identify some specific bugs in this patch, those can be
> fixed.

Sorry, it was https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167776.
Comment 25 Amit Mendapara CLA 2020-08-17 09:53:44 EDT
My bad. It was same.

BTW, the these changes https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167776/1/bundles/org.eclipse.ui.themes/css/e4_default_gtk.css should be reverted.
Comment 26 Mickael Istria CLA 2020-08-17 09:54:48 EDT
(In reply to Amit Mendapara from comment #25)
> My bad. It was same.
> 
> BTW, the these changes
> https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167776/1/bundles/
> org.eclipse.ui.themes/css/e4_default_gtk.css should be reverted.

Can you please share some before/after screenshots to explain what is degraded?
Comment 27 Amit Mendapara CLA 2020-08-17 10:28:10 EDT
Created attachment 283884 [details]
before.gif
Comment 28 Amit Mendapara CLA 2020-08-17 10:28:30 EDT
Created attachment 283885 [details]
after.gif
Comment 29 Amit Mendapara CLA 2020-08-17 10:29:40 EDT
Check the before and after gif. You can see the tabs have no separator borders when we mouse over an unselected tab.
Comment 30 Eclipse Genie CLA 2020-08-17 10:32:33 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167846
Comment 32 Pierre-Yves Bigourdan CLA 2020-09-01 05:19:47 EDT
Several third-party themes are now rendering incorrectly because of these changes (e.g. Spectrum theme and Planet themes), as the default behaviour of CTabRendering has changed with commit 3a36645ba15be9300cec478904018137f51eaccb. Could the new "swt-draw-custom-tab-content-background" CSS property please be documented in the N&N? It would have saved me a fair bit of debugging and investigation to narrow down to this commit. :)