Community
Participate
Working Groups
Currently, with CSS customizing CTabFolders, it seem impossible to have some line/border which goes around a CTabItem, including tab header and content of the tab area. This is definitely missing as there are colors for many other things like "outer keyline" which can sometime draw more than desired. The outline should probably also draw the border of the tab content.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167369
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/167369 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=d90ea661d5b01c2797b223051d12834d87c8c901
I've updated to the 4.18 M1 release this week, and I'm seeing a lot of problems which seem to have been caused by this change: * the Light theme now has a 2-pixel thick top border instead of a single pixel for other borders. * the Dark theme has a truncated pixel on the bottom left of the blue tab outline, and some borders are 2-pixel thick as with the Light theme. * the Classic theme has a missing pixel rendered in the top left of selected tabs. * rendering has changed quite significantly in my Planet themes plugin, it's no longer possible to not have the horizontal tab borders without the vertical ones. * Andrew's Spectrum theme is also impacted in odd ways, for example the left-most selected tab goes one pixel further than the tab folder border itself. * I haven't checked Clean Sheet or DevStyle available on the marketplace, but they may be impacted as well. Apart from the Classic theme issue which I've only been able to reproduce on macOS, all others I can reproduce on both Windows and macOS. This change probably needs to be reverted. If you're making big changes to accommodate the new System theme and need to modify the behaviour of existing CSS properties, it would be better to create a brand new CSS option alogether, or simply have a new CTabSystemRendering class which you point with the swt-tab-renderer CSS property. :)
I was able to reproduce some of the bugs mentioned by Pierre-Yves on Linux, although I didn't try today's IBuild with the classic theme. I'll add some screenshots.
Created attachment 284488 [details] Dark theme blue highlight truncated on bottom left > * the Dark theme has a truncated pixel on the bottom left of the blue tab
Created attachment 284489 [details] Light theme's top border is 2px thick > * the Light theme now has a 2-pixel thick top border instead of a single
Created attachment 284490 [details] Spectrum theme tab extends too far on the left > * Andrew's Spectrum theme is also impacted in odd ways, for example the > left-most selected tab goes one pixel further than the tab folder border > itself.
Looking at it, it seems DevStyle remains unaffected. I'm almost certain they define their own custom tab renderer however, and thus wouldn't be affected by any changes to the CTabRenderer in Platform.
@Pierre-Yves: the system theme is a different CSS, but it tries to reuse the CTabRendering that's common to other themes. I think we should really target having only 1 CTabRendering that works for everyone, ie fix bugs in it. However, the CTabRendering is an horrible piece of code to deal with, and it's hard to fix bugs without breaking things or to anticipate the potential issues a change may cause, and there is practically no easy way to easily automate tests for that AFAIK. I'll have a look at the issues you mention some time later. Any assistance would be welcome.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/171147
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/171147 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=b0d0f65bda821dace42a44b4d9c60f4974ba175b
(In reply to Eclipse Genie from comment #10) > New Gerrit change created: > https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/171147 This was merged as it fixes issue with the Light Theme (and probably some others). However, some work remain to be done: the selected tab background is 1 pixel too thin, its height should be 1 more pixel to better work with System theme, and I believe it's also the root cause of the bug identified in Dark Theme.
Will do some more testing with next nightly build.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/171233
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/171233 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=44525fb902ee9d8758a1dc864fb817d05b737d7b
(In reply to Eclipse Genie from comment #15) > Gerrit change > https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/171233 was merged > to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=44525fb902ee9d8758a1dc864fb817d05b737d7b Will wait for this to be in the nightly builds as well, not sure that it is in today's. :)
Created attachment 284584 [details] Separator between tabs in the light theme In the light theme, the tab separator is partly shadowed by another line.
Created attachment 284585 [details] Thick top and trauncate highlight in dark theme In the dark theme, the selected tab has a double-pixel border on top, and one pixel of the blue highlight is truncated.
Note that the two previous screenshots come from I20201026-1850 freshly downloaded from download.eclipse.org.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/171615
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/171615 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a4dc147577aa9277c4435e8028f518ad0340012d
Reported issues should now be fixed. Feel free to reopen if further issues are found; or to mark as VERIFIED is everything seems correct now.
Unfortunately there still seem to be quite a few problems.
Created attachment 284658 [details] Line below tab in dark theme In the dark theme, when the tab buttons are wrapped there is a line separator whereas there isn't when the button's are not wrapped. Not sure which is the expected behaviour here, but with Eclipse 4.17 there always was a line, this inconsistency is somewhat odd.
Created attachment 284659 [details] Spectrum theme In the Spectrum theme, there are still off-by-one pixel errors.
Created attachment 284660 [details] Planet themes In all Planet themes, the expected behaviour is just to have the top and bottom lines on selected tab folders, thanks to 0 pixel paddings on the right and left borders. This no longer works, the right and left lines are now systematically drawn, and the bottom line has become double pixel.
(In reply to Pierre-Yves B. from comment #26) > In all Planet themes, the expected behaviour is just to have the top and > bottom lines on selected tab folders, thanks to 0 pixel paddings on the I think this behavior was mostly relying on a bug or unspecified behavior. Padding has never been meant to crop or hode some borders. If we want finer control of whether to draw line on the sides, we need to do that via another CSS attribute, for example adding support for "border". Currently, the supported concepts regarding borders are outer keyline, outline, and inner keyline, which are not semantically affected by padding. As an intermediary steps, it would be great if we could just "disable" some of those lines by setting the color to null, or some size to 0px. However, it's IMO another piece of work for another enhancement request.
(In reply to Mickael Istria from comment #27) > I think this behavior was mostly relying on a bug or unspecified behavior. > Padding has never been meant to crop or hode some borders. If we want finer > control of whether to draw line on the sides, we need to do that via another > CSS attribute, for example adding support for "border". > Currently, the supported concepts regarding borders are outer keyline, > outline, and inner keyline, which are not semantically affected by padding. > As an intermediary steps, it would be great if we could just "disable" some > of those lines by setting the color to null, or some size to 0px. However, > it's IMO another piece of work for another enhancement request. Probably worth clarifying how this works in Planet themes. I'm using padding coupled with the swt-selected-tab-fill property. Having a one pixel padding at the bottom means that the content area starts at a distance of one pixel from the outer rim of the selected tab, displaying a one pixel line corresponding to the underlying filling of the tab. On left and right, I've got a zero pixel padding, effectively closing the gap between the content area and the tab's outer rim: the filling is therefore hidden and no lines will appear on either side. This really doesn't seem like a bug or undefined behaviour to me, it's actually how the padding CSS property works in browsers. The tab renderer still actually behaves like this today, all is good for the padding property. The problem really boils down to swt-tab-outline which is being used for the top border. The initial comment in this bug report states that "The outline should probably also draw the border of the tab content", and that to me is wrong. I'm not arguing about what "tab outline" semantically means in English, I frankly haven't given it much thought. My main points are: * "swt-tab-outline" was behaving like this for many years, probably ever since it was created. * no one ever reported it as a bug or complained about its behaviour. * there's probably a whole wealth of historical StackOverflow posts or forum threads which no longer make sense following this change. * people have leveraged old "swt-tab-outline" and built great Eclipse themes, either shipped with the Platform or third-party. Due to all the points above combined, it really shouldn't have changed IMHO, especially in such a fundamental way, i.e. drawing four lines instead of just one. And even if really it needed to change at all costs to improve the semantics, I think new CSS properties to enable the old behaviour should have been created upfront and communicated as soon as possible in the N&N, to minimise disruption. I'm happy to keep on helping with testing, but I'm getting quite nervous as M3 is approaching and there are still oddities in some of the built-in themes and at least two third-party themes are still badly impacted. :)
So do I get it right that basically, what you're actually willing for the Planet them is just to not draw any outline and just use padding to draw 1 line above and below the payload area? If so I suggest we add such capability in the renderer with proper elements rather than building such cases on top of side-effects of such implementation. For the record, about all changes that have happened to that class did break something in the past, and the behavior of such is not specified nor has tests to cover what's expected. So given this (absence of) contract, supporting all the variations of workarounds that were implemented on top of side-effects is not affordable. Ie the choices made by downstream themes cannot be taken into account in development of CTabRendering. All that said, we can keep working on improving CTabRendering so it fits everyone better; even if it involves getting rid of some of the work that was produced here. But it will need more that testing; code contribution will be necessary (I have the impression adding support for the various line "width" allowing them to be 0px would basically allow a proper and powerful enough solution to cover everything).
Note that in your case, if you want to keep playing with side-effects :P, you can probably set the taboutline color to the background/outerkeyline color and also add some padding-top to get the line drawn at the top.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/171806 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=47e77abc7ac5fd9d10160b3b191f0b93ced86f67
(In reply to Mickael Istria from comment #29) > If so I suggest we add such capability in the renderer with proper elements > rather than building such cases on top of side-effects of such > implementation. For my specific case, that would work, but I don't know if it would fix the other problems reported, in particular Andrew's Spectrum theme. (In reply to Mickael Istria from comment #30) > Note that in your case, if you want to keep playing with side-effects :P, > you can probably set the taboutline color to the background/outerkeyline > color and also add some padding-top to get the line drawn at the top. That wouldn't work, the line at the top isn't straight, it should outline the selected tab.
I think Themes built in Platform are fine, and the (almost non-existent) contract of CTabRendering are still respected. So let's mark it as resolved. Further issues or enhancement would better be reported in new tickets.
Hi Mickael, I got an email expressing frustration with the state of this bug. It seems that this change broke multiple external themes. Can we do something to allow the old behavior for the external themes?
.
(In reply to Lars Vogel from comment #34) > Hi Mickael, I got an email expressing frustration with the state of this > bug. It seems that this change broke multiple external themes. Can we do > something to allow the old behavior for the external themes? Reporting specific bugs would help.
(In reply to Mickael Istria from comment #36) > (In reply to Lars Vogel from comment #34) > > Hi Mickael, I got an email expressing frustration with the state of this > > bug. It seems that this change broke multiple external themes. Can we do > > something to allow the old behavior for the external themes? > > Reporting specific bugs would help. As far as I understand it, you change existing behavior in this bug, so the right place to change it back IMHO is this bug. My understanding is that the main concern is the following (quote): --------- The problem really boils down to swt-tab-outline which is being used for the top border. The initial comment in this bug report states that "The outline should probably also draw the border of the tab content", and that to me is wrong. I'm not arguing about what "tab outline" semantically means in English, I frankly haven't given it much thought. My main points are: * "swt-tab-outline" was behaving like this for many years, probably ever since it was created. * no one ever reported it as a bug or complained about its behaviour. * there's probably a whole wealth of historical StackOverflow posts or forum threads which no longer make sense following this change. * people have leveraged old "swt-tab-outline" and built great Eclipse themes, either shipped with the Platform or third-party. Due to all the points above combined, it really shouldn't have changed IMHO, especially in such a fundamental way, i.e. drawing four lines instead of just one. And even if really it needed to change at all costs to improve the semantics, I think new CSS properties to enable the old behaviour should have been created upfront and communicated as soon as possible in the N&N, to minimise disruptio ---------- So the question is, could the old behavior restored and your new desired behavior archived via a new property (maybe an additional flag)? I think Pyves and Andrews themes (and others) are a very nice addition to the Eclipse ecosystem and it would be nice if the do not break them if avoidable.
(In reply to Lars Vogel from comment #37) > So the question is, could the old behavior restored It would now be difficult to identify and revert the patches that are tied to it. And this would make the system theme totally fail, while it's IMO now a killer feature of the IDE. > and your new desied > behavior archived via a new property (maybe an additional flag)? Probably, but that would need some more effort to take care of it. My backlog is currently full, and I am actually reluctant in making effort to re-enable some previously undefined and faulty behaviour, even if it breaks some adopters. Indeed, according to https://www.w3schools.com/css/css_outline.asp , the "new" outline is mapping more correctly the previous one. I can add a note in the migration notes about outline now behaving differently a describe some possible hacks to workaround it in themes that have built on the previous behavior. There are still inner/outer keyline that can help. On the long run, if we want to avoid such issues, we need to really design and document how this rendering happen and how CSS properties are impacted. This code is basically a pile of workarounds, so any change -even for the better- can break some adopter.
We can probably find ways to get some of the old themes work without change by adding some more workarounds to the code (...), however, to evaluate what's feasible and what's not, we mostly need for each issue * the theme .css files used * a before vs after screenshot * a description that emphasize what are the specific points with the change that are perceived as a regression.
Moved to 4.19 M1, Please re target accordingly.
Mass move 4.19 M1 bugs to M3
Removing milestone from 4.19 M3 to 4.19, please re-target accordingly.
Mass move out of 4.19
Mass Move out of 4.20
