[
Date Prev][
Date Next][
Thread Prev][
Thread Next][
Date Index][
Thread Index]
[
List Home]
|
Re: [eclipse-pmc] Approval for Bug 516114 to add support for styling of the tabbed properties view
|
Alex, do you have objections?
Best regards, Lars
On Thu, May 11, 2017 at 6:12 PM, Oberhuber, Martin
<Martin.Oberhuber@xxxxxxxxxxxxx> wrote:
> Hi Dani, all -
>
>
>
> Thanks very much for your diligent checking, and asking hard questions.
>
>
>
> I am deeply convinced that maintaining the very high quality standards of
> the Eclipse Platform is key. Maintaining this requires sticking to plans and
> saying “no” to things even if they look compelling. On the other hand, there
> is genuine interest and the contribution looks solid to me from what I have
> seen (though I don’t really know the domain of e4/CSS so have to trust Lars’
> statements regarding the risk and possible implications).
>
>
>
> I’ve had a quick look, and we are talking about 17 files (some of them new)
> with approx. 380 LOC code plus approx. 180 LOC tests. To me, this looks like
> a complexity that is manageable. Given Lars’ explanations I am +1 for this
> change – would have been better to get into M7 candidate to make the test
> pass. As it appears to be *new* functionality affecting the dark theme only
> I am OK if submitter is testing on all arches. I’ll probably switch my own
> dev. Env to dark theme once I get hold of a build that includes this.
>
>
>
> Bottomline, +1 from me but this is *not* a precedent for sneaking stuff in
> that late and I’ll rather say no next time.
>
>
>
> Thanks,
>
> Martin
>
> --
>
> Martin Oberhuber, SMTS / Product Owner – Development Tools, Wind River
>
>
>
> From: <eclipse-pmc-bounces@xxxxxxxxxxx> on behalf of
> "daniel_megert@xxxxxxxxxx" <daniel_megert@xxxxxxxxxx>
> Reply-To: "eclipse-pmc@xxxxxxxxxxx" <eclipse-pmc@xxxxxxxxxxx>
> Date: Thursday 11 May 2017 at 15:25
> To: "eclipse-pmc@xxxxxxxxxxx" <eclipse-pmc@xxxxxxxxxxx>
>
>
> Subject: Re: [eclipse-pmc] Approval for Bug 516114 to add support for
> styling of the tabbed properties view
>
>
>
> 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
>
>
>
>
> _______________________________________________
> 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
