[GitHub] [ofbiz-framework] danwatford commented on a diff in pull request #548: Improved: Use FlexibleStringExpander for disabled attribute on fields

[GitBox <gi...@apache.org>]

danwatford commented on code in PR #548:
URL: https://github.com/apache/ofbiz-framework/pull/548#discussion_r986044913


##########
framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormField.java:
##########
@@ -483,10 +483,17 @@ public String getConditionGroup() {
         return conditionGroup;
     }
 
-    public boolean getDisabled() {
+    public FlexibleStringExpander getDisabled() {
         return disabled;
     }
 
+    public String getDisabled(Map<String, Object> context) {

Review Comment:
   Can we have a boolean returned here?   ModelFormField is the authority on whether a field is disabled based on context. If the property value is empty then ModelFormField can consider the field as enabled.



##########
framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormField.java:
##########
@@ -2738,25 +2745,30 @@ public ModelForm getModelGrid(Map<String, Object> context) {
      */
     public static class HiddenField extends FieldInfo {
         private final FlexibleStringExpander value;
+        private final FlexibleStringExpander disabled;

Review Comment:
   Is the disabled property relevant for hidden fields? If not then this is a chance to remove a few lines of code.



##########
framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormField.java:
##########
@@ -483,10 +483,17 @@ public String getConditionGroup() {
         return conditionGroup;
     }
 
-    public boolean getDisabled() {
+    public FlexibleStringExpander getDisabled() {

Review Comment:
   Perhaps rename this method to something like `getDisabledSpec` to indicate that it now contains the specification of how the enabled/disabled state of a field will be determined.
   
   If possible, keep this field private as caller should only really care if the field is disabled or not based on a given context.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ofbiz.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org