[
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
|
On Thu, May 11, 2017 at 11:20 PM, Lars Vogel <lars.vogel@xxxxxxxxxxx> wrote:
> Alex, do you have objections?
Just looked at the patch and tested it. Fixes really annoying issue so
it's worth having it although it comes a bit late. The changes are not
too invasive so we should be good.
+1 to have it for RC1.
>
> 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
> _______________________________________________
> 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
--
Alexander Kurtakov
Red Hat Eclipse Team
