Thanks Pierre,

the technical part looks good to me at first glance, I would though find it more appropriate to enhance the existing documentation set up by Adrian [1] instead of introducing another wiki page.

The last part contains suggestions to the reader which do not adhere to the projects view, namely the intention to reuse labels from other applications instead of using context related specified labels in the application itself. This was discussed between us and with other contributors many times in the past and you still insist to do it otherwise.

For reference, I point interested people to some of these discussions and issues from the past: [2] [3] [4]. There are more.

Some of the arguments to use specified labels instead of common labels and/or labels from other applications are:

- doing otherwise removes the flexibility for users to use different translations in different contexts. If you change the translation in this one place, it will also be changed anywhere else,

- even in a single installation of OFBiz, the same field might be translated differently between applications because they are used by different departments with their own terminology (true especially for bigger companies),

- in English, you often can use the same translation but in other languages, words are very different based on the context. Even the number of words differ,

- you would have to reference UI label maps from other applications which should be avoided because of (circular) dependencies. If we already have those situations, we should not extend it further.


So I propose that the project gives a clear understanding that

- specific labels should be used instead of common labels or reusing labels from other applications,

- new specific labels should be created instead of common labels be used,

- common labels CAN be used for action widgets like buttons (search, edit, delete) etc. which are the same in every context. This has to be checked though against languages

- changes in patches or pull requests, which do not align with those guidelines, should not be merged to the codebase.


Thanks,

Michael Brohl

ecomify GmbH - www.ecomify.de


[1] https://cwiki.apache.org/confluence/display/OFBIZ/Internationalization?src=contextnavchildmode

[2] https://issues.apache.org/jira/browse/OFBIZ-10772 (and subtasks)

[3] https://issues.apache.org/jira/browse/OFBIZ-8110?jql=project%20%3D%20OFBIZ%20AND%20text%20~%20%22Maximise%20utilisation%20of%20common%20labels%22

[4] https://lists.apache.org/thread/47lz9mytw2p7zzryddogt1283kkmkz2c

Am 29.01.22 um 18:28 schrieb Pierre Smits:
Hi all,

Over the years, contributions of translation definitions and contributions
to apply or change translation definitions in field and other widgets have
led to heated right-vs-wrong debates between contributors in tickets and
elsewhere. In order to unify the community, to decrease the number of these
heated debates and bring us back to happy collaborating to improve OFBiz,
  I have writtena guideline regarding this subject.

This guideline can be found here:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=199533364

I invite all to evaluate this guideline and provide feedback

Met vriendelijke groet,

Pierre Smits
*Proud* *contributor** of* Apache OFBiz <https://ofbiz.apache.org/> since
2008 (without privileges)
Proud contributor to the ASF since 2006
*Apache Directory <https://directory.apache.org>, PMC Member*

Anyone could have been you, whereas I've always been anyone.


On Fri, Jan 28, 2022 at 4:24 PM Daniel Watford <[email protected]> wrote:

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>

--
Daniel Watford

Reply via email to