Re: [eclipse-pmc] Approval for Bug 516114 to add support for styling of

["Daniel Megert" <daniel_megert@xxxxxxxxxx>]

Thanks for the details. For me it's still
quite some new code for RC1.

I will abstain from the vote and like
to hear what other PMC members say.

Dani



From:      
 Lars Vogel <lars.vogel@xxxxxxxxxxx>
To:      
 eclipse-pmc@xxxxxxxxxxx
Date:      
 10.05.2017 18:43
Subject:    
   Re: [eclipse-pmc]
Approval for Bug 516114 to add support for styling of the tabbed properties
view
Sent by:    
   eclipse-pmc-bounces@xxxxxxxxxxx

---

On Wed, May 10, 2017 at 6:26 PM, Daniel Megert <daniel_megert@xxxxxxxxxx>
wrote:
> A lot of stuff is called by the CSS engine ;-)

Indeed. :-)

> Which CSS property? Where do I see this restriction in the code and
that it
> is restricted to the Dark theme?

The new properties are in the change. See
https://git.eclipse.org/r/#/c/96470/6/bundles/org.eclipse.ui.views.properties.tabbed/plugin.xml

If the CSS engine finds such a property for a registered CSS element
(in most cases widgets, but we also have other handlers for example
for preferences), it will call the adapter to adapt the widget to the
CSS engine. The registered handler will afterwards be called. A
handler is also registered via the plugin.xml.

In Matthias case we have two handlers,
https://git.eclipse.org/r/#/c/96470/6/bundles/org.eclipse.ui.views.properties.tabbed/src/org/eclipse/ui/internal/views/properties/tabbed/css/TabbedPropertyListCssPropertyHandler.java
and https://git.eclipse.org/r/#/c/96470/6/bundles/org.eclipse.ui.views.properties.tabbed/src/org/eclipse/ui/internal/views/properties/tabbed/css/TabbedPropertyTitleCssPropertyHandler.java

Based on your concerns I asked Matthias via the Gerrit to add a test
that if the CSS property is not set that the default colors are not
changed.

Best regards, Lars


>
> Dani
>
>
>
> From:        Lars Vogel <lars.vogel@xxxxxxxxxxx>
> To:        eclipse-pmc@xxxxxxxxxxx
> Date:        10.05.2017 18:13
> Subject:        Re: [eclipse-pmc] Approval for
Bug 516114 to add support for
> styling of the tabbed properties view
> Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx
> ________________________________
>
>
>
> These new internal methods are only called by the CSS engine ->
no
> influence on the other themes if the CSS property is not set.
>
>
>
>
>
> On Wed, May 10, 2017 at 6:10 PM, Daniel Megert <daniel_megert@xxxxxxxxxx>
> wrote:
>> Please explain which changes e.g. in TabbedPropertyList.java only
affect
>> the
>> dark theme? And also in all the other Java classes.
>>
>> Dani
>>
>>
>>
>> From:        Lars Vogel <lars.vogel@xxxxxxxxxxx>
>> To:        eclipse-pmc@xxxxxxxxxxx
>> Date:        10.05.2017 18:05
>> Subject:        Re: [eclipse-pmc] Approval
for Bug 516114 to add support
>> for
>> styling of the tabbed properties view
>> Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx
>> ________________________________
>>
>>
>>
>> On Wed, May 10, 2017 at 6:00 PM, Daniel Megert <daniel_megert@xxxxxxxxxx>
>> wrote:
>>>> Dani, the patch will only influence the dark theme.
>>>
>>> I strongly disagree. None of the code changes I quickly looked
at are
>>> restricted to a specific theme.
>>
>> Yes, they are. They way this works in the CSS engine is via the
CSS
>> file. See that in https://git.eclipse.org/r/#/c/96470/onlythedark
>
>>
>> theme css has changed
>> (bundles/org.eclipse.ui.themes/css/dark/e4-dark_globalstyle.css).
All
>> other css files are unchanged.
>>
>>
>>
>>>
>>> Dani
>>>
>>>
>>>
>>> From:        Lars Vogel <lars.vogel@xxxxxxxxxxx>
>>> To:        eclipse-pmc@xxxxxxxxxxx
>>> Date:        10.05.2017 17:52
>>> Subject:        Re: [eclipse-pmc] Approval
for Bug 516114 to add support
>>> for
>>> styling of the tabbed properties view
>>>
>>> Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx
>>> ________________________________
>>>
>>>
>>>
>>> Dani, the patch will only influence the dark theme. The CSS
engine is
>>> implemented in a way that it only applies a CSS handler  if
a CSS
>>> property
>>> is set. The patch from Matthias define styling only for the
dark theme.
>>>
>>> Verification has been done on Windows and Linux. The CSS engine
does rely
>>> on
>>> setters of the individual SWT widgets, so any giving limitation
for SWT
>>> widgets will also apply for the CSS engine.
>>>
>>> Matthias, do you have access to a Mac to validate the changes
on a Mac?
>>>
>>>> To me the change looks too big for RC1 and I'd vote to
move it to 4.7.1.
>>>
>>> I think this moves it to 4.7.1 unless Dani changes his mind.
>>>
>>> Best regards, Lars
>>>
>>>
>>> Am 10.05.2017 5:21 nachm. schrieb "Daniel Megert"
>>> <daniel_megert@xxxxxxxxxx>:
>>> Did you verify the change on all platforms and all themes?
>>>
>>> To me the change looks too big for RC1 and I'd vote to move
it to 4.7.1.
>>>
>>> Dani
>>>
>>>
>>>
>>> From:        "Becker, Matthias"
<ma.becker@xxxxxxx>
>>> To:        "eclipse-pmc@xxxxxxxxxxx"
<eclipse-pmc@xxxxxxxxxxx>
>>> Date:        10.05.2017 08:58
>>> Subject:        [eclipse-pmc] Approval
for Bug 516114 to add support for
>>> styling of the tabbed properties view
>>> Sent by:        eclipse-pmc-bounces@xxxxxxxxxxx
>>> ________________________________
>>>
>>>
>>>
>>> Dear PMC,
>>>
>>> I would like to ask for PMC permissions for “Bug 516114-
[CSS] [Dark] Add
>>> support for styling of TabbedProperties in Properties View
(and apply to
>>> dark layout)”.
>>>
>>> This fix is need to provide the CSS styling implementation
for the tabbed
>>> properties view and to provide the css for the dark theme.
The tabbed
>>> properties view is used in the “Resource” perspective and
also in SAPs
>>> “ABAP
>>> Development Tools” (
>>
> https://tools.hana.ondemand.com/#abap).
As the “ABAP
>
>>
>>> Development Tools” use the tabbed properties view extensively
(I think
>>> other
>>> products do as well) our Team currently cannot provide support
for the
>>> dark
>>> theme.I really would like to have this in Oxygen so that Eclipse
can
>>> deliver
>>> a relative decent dark theme with Oxygen. In addition, it’s
my plan to
>>> provide the first support of the dark theme in the “ABAP
Development
>>> Tools”
>>> in our Oxygen-based version.
>>>
>>> The implementation took the implementation of “Bug 465148-
[CSS] [Dark]
>>> Add
>>> support for styling Forms (and apply to dark layout)” as
a blueprint. The
>>> change also contains Unit tests. The changes are limited to
the
>>> org.eclipse.ui.views.properties.tabbed bundle. Hence the risk
should be
>>> low.
>>> If we do not deliver it we have an inconsistency in our UI.
>>>
>>> This is my first code contribution to the eclipse platform
ui project. Up
>>> to
>>> now I only have contributed a HiDPI Icons to various eclipse
projects.
>>> This
>>> change still needs the final code review by the Platform UI
team. Lars
>>> Vogel
>>> is willing to do a detailed code review after PMC approval.
>>>
>>> Best regards,
>>> Matthias Becker
>>> (SAP SE)_______________________________________________
>>> eclipse-pmc mailing list
>>> eclipse-pmc@xxxxxxxxxxx
>>> To change your delivery options, retrieve your password, or
unsubscribe
>>> from
>>> this list, visit
>>>
>>
> https://dev.eclipse.org/mailman/listinfo/eclipse-pmc
>
>>
>>>
>>>
>>>
>>> _______________________________________________
>>> eclipse-pmc mailing list
>>> eclipse-pmc@xxxxxxxxxxx
>>> To change your delivery options, retrieve your password, or
unsubscribe
>>> from
>>> this list, visit
>>>
>>>
>>> https://dev.eclipse.org/mailman/listinfo/eclipse-pmc_______________________________________________
>>> eclipse-pmc mailing list
>>> eclipse-pmc@xxxxxxxxxxx
>>> To change your delivery options, retrieve your password, or
unsubscribe
>>> from
>>> this list, visit
>>> https://dev.eclipse.org/mailman/listinfo/eclipse-pmc
>>>
>>>
>>>
>>> _______________________________________________
>>> eclipse-pmc mailing list
>>> eclipse-pmc@xxxxxxxxxxx
>>> To change your delivery options, retrieve your password, or
unsubscribe
>>> from
>>> this list, visit
>>> https://dev.eclipse.org/mailman/listinfo/eclipse-pmc
>>
>>
>>
>> --
>> Eclipse Platform UI and e4 project co-lead
>> CEO vogella GmbH
>>
>> Haindaalwisch 17a, 22395 Hamburg
>> Amtsgericht Hamburg: HRB 127058
>> Geschäftsführer: Lars Vogel, Jennifer Nerlich de Vogel
>> USt-IdNr.: DE284122352
>> Fax (040) 5247 6322, Email: lars.vogel@xxxxxxxxxxx, Web:
>> http://www.vogella.com
>> _______________________________________________
>> eclipse-pmc mailing list
>> eclipse-pmc@xxxxxxxxxxx
>> To change your delivery options, retrieve your password, or unsubscribe
>> from
>> this list, visit
>> https://dev.eclipse.org/mailman/listinfo/eclipse-pmc
>>
>>
>>
>> _______________________________________________
>> eclipse-pmc mailing list
>> eclipse-pmc@xxxxxxxxxxx
>> To change your delivery options, retrieve your password, or unsubscribe
>> from
>> this list, visit
>> https://dev.eclipse.org/mailman/listinfo/eclipse-pmc
>
>
>
> --
> Eclipse Platform UI and e4 project co-lead
> CEO vogella GmbH
>
> Haindaalwisch 17a, 22395 Hamburg
> Amtsgericht Hamburg: HRB 127058
> Geschäftsführer: Lars Vogel, Jennifer Nerlich de Vogel
> USt-IdNr.: DE284122352
> Fax (040) 5247 6322, Email: lars.vogel@xxxxxxxxxxx, Web:
> http://www.vogella.com
> _______________________________________________
> eclipse-pmc mailing list
> eclipse-pmc@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe
from
> this list, visit
> https://dev.eclipse.org/mailman/listinfo/eclipse-pmc
>
>
>
> _______________________________________________
> eclipse-pmc mailing list
> eclipse-pmc@xxxxxxxxxxx
> To change your delivery options, retrieve your password, or unsubscribe
from
> this list, visit
> https://dev.eclipse.org/mailman/listinfo/eclipse-pmc



-- 
Eclipse Platform UI and e4 project co-lead
CEO vogella GmbH

Haindaalwisch 17a, 22395 Hamburg
Amtsgericht Hamburg: HRB 127058
Geschäftsführer: Lars Vogel, Jennifer Nerlich de Vogel
USt-IdNr.: DE284122352
Fax (040) 5247 6322, Email: lars.vogel@xxxxxxxxxxx, Web: http://www.vogella.com
_______________________________________________
eclipse-pmc mailing list
eclipse-pmc@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe
from this list, visit
https://dev.eclipse.org/mailman/listinfo/eclipse-pmc