Hi Dan,

thanks for your explanations! I see your point and generally agree with your approach and summary, that a common label might be better than none.

Just a hint why I am so strong in my opinion here: as you can see in the following thread, Pierre is using the fact that those changes get into the codebase to justify his approach to generally use common labels/reuse labels from other applications and to prove that this is the right way to go.

I am confident that your reviews are thorough and thank you for your work.

Best regards,

Michael

Am 28.01.22 um 16:24 schrieb Daniel Watford:
Hi Michael,

Yes, those labels were intentional. My view was that introducing common
labels in cases where there were previously no labels at all was an
improvement.

Had there already been specific labels in place, these would have been left
alone.

Summary of my review approach:
- Changing from application specific labels to common labels is unlikely to
be committed without good reason because of the risk of losing the original
application's intention when translated to different languages.
- Changes from no label at all to a common label seem reasonable since we
have the potential benefit of already existing generic translations from
the common label set.

Of course there will be exceptions to every rule, but in this case I
thought the changes appropriate and an improvement.

Thanks,

Dan.


On Fri, 28 Jan 2022 at 14:38, Michael Brohl <[email protected]>
wrote:

Hi Dan,

is this commit intentional?

It still has unwanted changes introducing titles with common labels
instead of specific labels (e.g. CommonLocation for jobLocation).

Regards,

Michael


Am 28.01.22 um 08:49 schrieb [email protected]:
This is an automated email from the ASF dual-hosted git repository.

danwatford pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git


The following commit(s) were added to refs/heads/trunk by this push:
       new b2dd912  Improved: Recruitment screens updated to use Grid
rather than list forms (OFBIZ-11345)
b2dd912 is described below

commit b2dd912794f083a6176dbd581967e2f97c3c183a
Author: Pierre Smits <[email protected]>
AuthorDate: Fri Jan 28 08:49:01 2022 +0100

      Improved: Recruitment screens updated to use Grid rather than list
forms (OFBIZ-11345)

      According to the definition in widget-form.xsd the
      use of a combination of a form with type="list" is
      deprecated in favour of a grid. Therefore refactored various
      Recruitment forms to use grid and updated references from
      relevant screens to forms.

      Thanks: Pierre Smits for changes.
---
   .../humanres/widget/RecruitmentScreens.xml         | 12 +++----
   .../humanres/widget/forms/RecruitmentForms.xml     | 42
+++++++++++-----------
   2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/applications/humanres/widget/RecruitmentScreens.xml
b/applications/humanres/widget/RecruitmentScreens.xml
index 7072f0a..2452265 100644
--- a/applications/humanres/widget/RecruitmentScreens.xml
+++ b/applications/humanres/widget/RecruitmentScreens.xml
@@ -20,7 +20,7 @@ under the License.
   <screens xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
       xmlns="http://ofbiz.apache.org/Widget-Screen"; xsi:schemaLocation="
http://ofbiz.apache.org/Widget-Screen
http://ofbiz.apache.org/dtds/widget-screen.xsd";>
       <screen name="FindJobRequisitions">
-        <section>
+        <section>
               <actions>
                   <set field="titleProperty"
value="PageTitleFindJobRequisition"/>
                   <set field="tabButtonItem" value="JobRequisition"/>
@@ -54,7 +54,7 @@ under the License.
                                           <include-form
name="FindJobRequisitions"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                       <decorator-section
name="search-results">
-                                        <include-form
name="ListJobRequisitions"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
+                                        <include-grid
name="ListJobRequisitions"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                   </decorator-screen>
                               </widgets>
@@ -124,7 +124,7 @@ under the License.
                                           <include-form
name="FindInternalJobPosting"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                       <decorator-section
name="search-results">
-                                        <include-form
name="ListInternalJobPosting"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
+                                        <include-grid
name="ListInternalJobPosting"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                   </decorator-screen>
                               </widgets>
@@ -187,7 +187,7 @@ under the License.
                                           <include-form
name="FindJobInterview"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                       <decorator-section
name="search-results">
-                                        <include-form
name="ListInterview"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
+                                        <include-grid
name="ListInterview"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                   </decorator-screen>
                               </widgets>
@@ -249,7 +249,7 @@ under the License.
                                           <include-form
name="FindApprovals"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                       <decorator-section
name="search-results">
-                                        <include-form
name="ListApprovals"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
+                                        <include-grid
name="ListApprovals"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                   </decorator-screen>
                               </widgets>
@@ -302,7 +302,7 @@ under the License.
                                           <include-form
name="FindRelocation"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                       <decorator-section
name="search-results">
-                                        <include-form
name="ListRelocation"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
+                                        <include-grid
name="ListRelocation"
location="component://humanres/widget/forms/RecruitmentForms.xml"/>
                                       </decorator-section>
                                   </decorator-screen>
                               </widgets>
diff --git a/applications/humanres/widget/forms/RecruitmentForms.xml
b/applications/humanres/widget/forms/RecruitmentForms.xml
index 7447a11..ccca4ed 100644
--- a/applications/humanres/widget/forms/RecruitmentForms.xml
+++ b/applications/humanres/widget/forms/RecruitmentForms.xml
@@ -59,8 +59,8 @@ under the License.
           <field name="qualification"><hidden/></field>
           <field name="searchButton" title="${uiLabelMap.CommonFind}"
widget-style="smallSubmit"><submit button-type="button"/></field>
       </form>
-    <form name="ListJobRequisitions" list-name="listIt" type="list"
odd-row-style="alternate-row" header-row-style="header-row-2"
-        paginate-target="FindJobRequisitions"
default-table-style="basic-table hover-bar">
+    <grid name="ListJobRequisitions" list-name="listIt"
paginate-target="FindJobRequisitions"
+        default-table-style="basic-table hover-bar"
odd-row-style="alternate-row" header-row-style="header-row-2">
           <actions>
               <set field="entityName" value="JobRequisition"/>
               <service service-name="performFind" result-map="result"
result-map-list="listIt">
@@ -70,19 +70,19 @@ under the License.
                   <field-map field-name="viewSize"
from-field="viewSize"/>
               </service>
           </actions>
-        <field name="jobRequisitionId" widget-style="buttontext"
use-when="hasAdminPermission">
+        <field name="jobRequisitionId" title="${uiLabelMap.CommonId}"
widget-style="buttontext" use-when="hasAdminPermission">
               <hyperlink description="${jobRequisitionId}"
target="EditJobRequisition">
                   <parameter param-name="jobRequisitionId"/>
               </hyperlink>
           </field>
-        <field name="jobRequisitionId"
use-when="!hasAdminPermission"><display/></field>
+        <field name="jobRequisitionId" title="${uiLabelMap.CommonId}"
use-when="!hasAdminPermission"><display/></field>
           <field name="skillTypeId">
               <display-entity entity-name="SkillType"/>
           </field>
           <field name="jobPostingTypeEnumId"><display/></field>
           <field name="examTypeEnumId"><display/></field>
           <field name="qualification"><display/></field>
-        <field name="jobLocation"><display/></field>
+        <field name="jobLocation"
title="${uiLabelMap.CommonLocation}"><display/></field>
           <field name="experienceYears"><display/></field>
           <field name="experienceMonths"><display/></field>
           <field name="applyLink" title="${uiLabelMap.CommonApply}"
widget-style="buttontext">
@@ -95,7 +95,7 @@ under the License.
                   <parameter param-name="jobRequisitionId"/>
               </hyperlink>
           </field>
-    </form>
+    </grid>
       <form name="EditJobRequisition" type="single"
target="updateJobRequisition" default-map-name="jobRequisition">
           <alt-target use-when="jobRequisition==null"
target="createJobRequisition"/>
           <field name="jobRequisitionId"
use-when="jobRequisition==null"><ignored/></field>
@@ -161,8 +161,8 @@ under the License.
           <field name="referredByPartyId"><hidden/></field>
           <field name="searchButton" title="${uiLabelMap.CommonFind}"
widget-style="smallSubmit"><submit button-type="button"/></field>
       </form>
-    <form name="ListInternalJobPosting" list-name="listIt" type="list"
odd-row-style="alternate-row" header-row-style="header-row-2"
-        paginate-target="FindInternalJobPosting"
default-table-style="basic-table hover-bar">
+    <grid name="ListInternalJobPosting" list-name="listIt"
paginate-target="FindInternalJobPosting"
+        odd-row-style="alternate-row" header-row-style="header-row-2"
default-table-style="basic-table hover-bar">
           <actions>
               <set field="entityName" value="EmploymentApp"/>
               <service service-name="performFind" result-map="result"
result-map-list="listIt">
@@ -177,16 +177,14 @@ under the License.
               <hyperlink description="${applicationId}"
target="EditInternalJobPosting">
                   <parameter param-name="applicationId"/>
               </hyperlink>
-        </field>
-
+        </field>
           <field name="approverPartyId">
               <display-entity entity-name="PartyNameView"
description="${firstName} ${lastName}" key-field-name="partyId">
                   <sub-hyperlink target="/partymgr/control/viewprofile"
target-type="inter-app" description="${approverPartyId}"
link-style="buttontext">
                       <parameter param-name="partyId"
from-field="approverPartyId"/>
                   </sub-hyperlink>
               </display-entity>
-        </field>
-
+        </field>
           <field name="applyingPartyId">
               <display-entity entity-name="PartyNameView"
description="${firstName} ${lastName}" key-field-name="partyId">
                   <sub-hyperlink target="/partymgr/control/viewprofile"
target-type="inter-app" description="${applyingPartyId}"
link-style="buttontext">
@@ -194,7 +192,6 @@ under the License.
                   </sub-hyperlink>
               </display-entity>
           </field>
-
           <field name="statusId"
title="${uiLabelMap.HumanResIJPStatus}"><display/></field>
           <field name="jobRequisitionId" widget-style="buttontext">
               <hyperlink description="${jobRequisitionId}"
target="EditJobRequisition">
@@ -209,7 +206,7 @@ under the License.
           <field name="emplPositionId"><hidden/></field>
           <field name="employmentAppSourceTypeId"><hidden/></field>
           <field name="referredByPartyId"><hidden/></field>
-    </form>
+    </grid>
       <form name="EditInternalJobPosting" type="single"
target="updateInternalJobPosting">
           <alt-target use-when="employmentApp==null"
target="createInternalJobPosting"/>
           <auto-fields-service service-name="updateInternalJobPosting"
map-name="employmentApp"/>
@@ -254,7 +251,8 @@ under the License.
           </field>
           <field name="searchButton" title="${uiLabelMap.CommonFind}"
widget-style="smallSubmit"><submit button-type="button"/></field>
       </form>
-    <form name="ListInterview" type="list" list-name="listIt"
paginate-target="FindJobInterview" odd-row-style="alternate-row"
header-row-style="header-row-2" default-table-style="basic-table hover-bar">
+    <grid name="ListInterview" list-name="listIt"
paginate-target="FindJobInterview"
+        odd-row-style="alternate-row" header-row-style="header-row-2"
default-table-style="basic-table hover-bar">
           <actions>
               <set field="entityName" value="JobInterview"/>
               <service service-name="performFind" result-map="result"
result-map-list="listIt">
@@ -289,7 +287,7 @@ under the License.
                   <parameter param-name="jobInterviewId"/>
               </hyperlink>
           </field>
-    </form>
+    </grid>
       <form name="EditJobInterview" type="single"
target="updateJobInterview">
           <actions>
               <entity-one entity-name="JobInterview"
value-field="JobInterview"/>
@@ -345,7 +343,7 @@ under the License.
           <field name="jobRequisitionId"><lookup
target-form-name="LookupJobRequisition"/></field>
           <field name="searchButton" title="${uiLabelMap.CommonFind}"
widget-style="smallSubmit"><submit button-type="button"/></field>
       </form>
-    <form name="ListApprovals" list-name="listIt" type="list"
odd-row-style="alternate-row" header-row-style="header-row-2"
default-table-style="basic-table hover-bar">
+    <grid name="ListApprovals" list-name="listIt"
odd-row-style="alternate-row" header-row-style="header-row-2"
default-table-style="basic-table hover-bar">
           <actions>
               <set field="entityName" value="EmploymentApp"/>
               <service service-name="performFind" result-map="result"
result-map-list="listIt">
@@ -363,7 +361,7 @@ under the License.
                   </sub-hyperlink>
               </display-entity>
           </field>
-        <field name="approverPartyId" field-name="partyId">
+        <field name="approverPartyId">
               <display-entity entity-name="PartyNameView"
description="${firstName} ${lastName}">
                   <sub-hyperlink target="/partymgr/control/viewprofile"
target-type="inter-app" description="${approverPartyId}"
link-style="buttontext">
                       <parameter param-name="partyId"
from-field="approverPartyId"/>
@@ -378,7 +376,7 @@ under the License.
           <field name="emplPositionId"><hidden/></field>
           <field name="employmentAppSourceTypeId"><hidden/></field>
           <field name="referredByPartyId"><hidden/></field>
-    </form>
+    </grid>
       <form name="EditApprovalStatus" type="single"
target="updateApprovalStatus" default-map-name="employmentApp">
           <auto-fields-service service-name="updateApprovalStatus"
map-name="employmentApp"/>
           <field name="applicationId"><display/></field>
@@ -417,7 +415,7 @@ under the License.
           <field name="noConditionFind"><hidden value="Y"/><!-- if this
isn't there then with all fields empty no query will be done --></field>
           <field name="searchButton" title="${uiLabelMap.CommonFind}"
widget-style="smallSubmit"><submit button-type="button"/></field>
       </form>
-    <form name="ListRelocation" list-name="listIt" type="list"
odd-row-style="alternate-row" header-row-style="header-row-2"
+    <grid name="ListRelocation" list-name="listIt"
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="result"
result-map-list="listIt">
@@ -446,7 +444,7 @@ under the License.
           </field>
           <field name="internalOrganisation"><display/></field>
           <field name="reportingDate"><display/></field>
-        <field name="location">
+        <field name="location" title="${uiLabelMap.CommonLocation}">
               <display description="${groovy:
                   import org.apache.ofbiz.entity.GenericValue;
                   import org.apache.ofbiz.base.util.UtilMisc;
@@ -457,5 +455,5 @@ under the License.
                   return city;
                   }"/>
           </field>
-    </form>
+    </grid>
   </forms>

Reply via email to