I agree with Jacques and Deepak. The code is an improvement on what was there, end of story. A contributor is a volunteer who is free to do as much or as little as they please so long as each contribution is a full step forward.
Regards Scott On 3/04/2017 17:13, "Deepak Dixit" <deepak.di...@hotwaxsystems.com> wrote: > I agree with Jacques, > it will also help to backport fixes to older releases. > If we mix them then it will difficult to identify the fix. > > > > Thanks & Regards > -- > Deepak Dixit > www.hotwaxsystems.com > > On Sun, Apr 2, 2017 at 5:28 PM, Taher Alkhateeb < > slidingfilame...@gmail.com> > wrote: > > > 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"> > > >>> > > >>> > > >>> > > >>> > > > > > >
