+1

It would be good to do refactoring/cleanup when code is changed or at least 
file a Jira when there is no time to do it all at once.

Let's apply this as a pattern and keep eyes open.

Regards,
Michael



> Am 02.04.2017 um 13:58 schrieb Taher Alkhateeb <slidingfilame...@gmail.com>:
> 
> Why not guide and mentor the contributor to change the list to a grid in
> the same commit? Why do the work twice?
> 
> If we use the logic of "not mix things" then anytime we work on a piece of
> code we don't refactor it because we should "not mix things". That just
> doesn't make a lot of sense to me. Anytime we apply a feature or fix for a
> bug or do anything then we should also include clean up and refactoring as
> part of that exercise IMO.
> 
> On Sun, Apr 2, 2017 at 2:33 PM, Jacques Le Roux <
> jacques.le.r...@les7arts.com> wrote:
> 
>> I prefer to not mix things in a patch from a contributor.
>> 
>> But yes the grid is slowly replacing the list form.
>> 
>> Jacques
>> 
>> 
>> 
>>> Le 02/04/2017 à 12:47, Taher Alkhateeb a écrit :
>>> 
>>> I am a little confused! You had many commits in the past where you are
>>> converting list-forms into grids (I even had some questions on that in a
>>> separate thread). Now you are refactoring a form and adding fields without
>>> converting it into a grid.
>>> 
>>> Are we switching to grids or not? or are we doing this only selectively?
>>> If
>>> no, why didn't we change it here? If yes, then what's the point of all the
>>> effort of changing these forms to grids?
>>> 
>>> Regards,
>>> 
>>> Taher Alkhateeb
>>> 
>>> On Sun, Apr 2, 2017 at 12:26 PM, <jler...@apache.org> wrote:
>>> 
>>> Author: jleroux
>>>> Date: Sun Apr  2 09:26:36 2017
>>>> New Revision: 1789863
>>>> 
>>>> URL: http://svn.apache.org/viewvc?rev=1789863&view=rev
>>>> Log:
>>>> Fixed: Sort Links in Lookup for Data Resource Id causes unwanted
>>>> behaviour
>>>> on
>>>> Find Content page
>>>> (OFBIZ-9280)
>>>> 
>>>> How to reproduce :
>>>> 1. Log in the Content component
>>>> 2. Go to the Content tab. https://localhost:8443/
>>>> content/control/findContent
>>>> 3. In the search form, open the lookup of the field ' Data Resource Id '
>>>> 4. Click on any of the table header links to sort the table. Results will
>>>> be
>>>>  displayed on a new unstyled window closing the Lookup dialog.
>>>> 
>>>> 
>>>> Problem:
>>>> Problem:
>>>> 1. Lookup dialog for dataResourceId uses "ListLookupDataResource" form to
>>>> list
>>>> Data Resource records.
>>>> 2. "ListLookupDataResource" form extends "ListDataResource" form for all
>>>> the
>>>> fields.
>>>> 3. Sort-field is set to true in "ListDataResource" form for all the
>>>> fields.
>>>> 4. Sort-field adds <a> hyperlink to "LookupResource" .
>>>> 5. When clicked <a> link it moves to a new page and doesn't renders the
>>>> response
>>>>  in the Lookup dialog.
>>>> Solution:
>>>> First of all it is a lookup to select DataResourceId so as the pattern
>>>> followed
>>>> in other lookups in OFBiz it should not have any sort-fields.To set it
>>>> false,
>>>> it should be overridden. If all fields needs to be overridden it is
>>>> better
>>>> not
>>>> to extend "ListDataResource" form and add these fields right away.
>>>> Apart from that added 'widget-style="smallSubmit"' to dataResourceId
>>>> according
>>>> to the uniform pattern followed.
>>>> 
>>>> jleroux: I added the header-row-style="header-row-2" style
>>>> 
>>>> Thanks: Aditya Sharma
>>>> 
>>>> Modified:
>>>>     ofbiz/ofbiz-framework/trunk/applications/content/widget/
>>>> content/DataResourceForms.xml
>>>> 
>>>> Modified: ofbiz/ofbiz-framework/trunk/applications/content/widget/
>>>> content/DataResourceForms.xml
>>>> URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/
>>>> applications/content/widget/content/DataResourceForms.xml?
>>>> rev=1789863&r1=1789862&r2=1789863&view=diff
>>>> ============================================================
>>>> ==================
>>>> --- ofbiz/ofbiz-framework/trunk/applications/content/widget/cont
>>>> ent/DataResourceForms.xml
>>>> (original)
>>>> +++ ofbiz/ofbiz-framework/trunk/applications/content/widget/cont
>>>> ent/DataResourceForms.xml
>>>> Sun Apr  2 09:26:36 2017
>>>> @@ -127,10 +127,10 @@ under the License.
>>>>                  <sort-field name="lastModifiedByUserLogin"/>
>>>>              </field-group>
>>>>          </sort-order>
>>>> -
>>>>      </form>
>>>> -    <form name="ListLookupDataResource" extends="ListDataResource"
>>>> list-name="listIt" type="list" paginate-target="LookupDataResource"
>>>> -        odd-row-style="alternate-row" default-table-style="basic-table
>>>> hover-bar">
>>>> +
>>>> +    <form name="ListLookupDataResource" list-name="listIt" type="list"
>>>> paginate-target="LookupDataResource"
>>>> +        odd-row-style="alternate-row" header-row-style="header-row-2"
>>>> default-table-style="basic-table hover-bar">
>>>>          <actions>
>>>>              <service service-name="performFind"
>>>> result-map-list="listIt">
>>>>                  <field-map field-name="inputFields"
>>>> from-field="parameters"/>
>>>> @@ -140,10 +140,16 @@ under the License.
>>>>                  <field-map field-name="viewSize"
>>>> from-field="viewSize"/>
>>>>              </service>
>>>>          </actions>
>>>> -        <field name="dataResourceId" title="${uiLabelMap.
>>>> ContentDataResourceId}">
>>>> +        <field name="dataResourceId" title="${uiLabelMap.ContentDat
>>>> aResourceId}"
>>>> widget-style="smallSubmit">
>>>>              <hyperlink description="${dataResourceId}"
>>>> target="javascript:set_value('${dataResourceId}')" also-hidden="false"
>>>> target-type="plain"/>
>>>>          </field>
>>>>          <field name="dataResourceName"><display/></field>
>>>> +        <field name="dataResourceTypeId"><display-entity
>>>> entity-name="DataResourceType"></display-entity></field>
>>>> +        <field name="mimeTypeId"><display-entity
>>>> entity-name="MimeType"></display-entity></field>
>>>> +        <field name="statusId"><display-entity
>>>> entity-name="StatusItem"></display-entity></field>
>>>> +        <field name="localeString"><display-entity
>>>> entity-name="CountryCode" description="${countryName}[${countryCode}]"
>>>> key-field-name="countryCode"></display-entity></field>
>>>> +        <field name="createdByUserLogin"><display-entity
>>>> also-hidden="false" entity-name="PartyNameView" key-field-name="partyId"
>>>> description="${groupName}${firstName} ${lastName}"/></field>
>>>> +        <field name="dataCategoryId"><display-entity
>>>> entity-name="DataCategory" description="${categoryName}[$
>>>> {dataCategoryId}]"></display-entity></field>
>>>>      </form>
>>>>      <form name="MruLookupDataResource" default-entity-name="DataResou
>>>> rce"
>>>> list-name="mruList" target="" title="" type="list"
>>>> default-widget-style="display"
>>>>          odd-row-style="alternate-row" default-table-style="basic-table
>>>> hover-bar">
>>>> 
>>>> 
>>>> 
>>>> 
>> 

Reply via email to