danwatford commented on code in PR #548:
URL: https://github.com/apache/ofbiz-framework/pull/548#discussion_r989905225
##########
framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormField.java:
##########
@@ -2799,6 +2811,26 @@ public String getValue(Map<String, Object> context) {
return getModelFormField().getEntry(context);
}
+ /**
+ * Gets disabled.
+ * @return the disabled
+ */
+ public FlexibleStringExpander getDisabled() {
Review Comment:
Can this be renamed `getDisabledSpec` to differentiate it from calling
`getDisabled` with context arguments. (Similar to the change made to the
`ModelFormField` class above.)
Does anything use this method? If not, we should remove it or make it
private.
##########
framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormFieldBuilder.java:
##########
@@ -729,7 +729,7 @@ public String getConditionGroup() {
* Gets disabled.*
* @return the disabled
*/
- public boolean getDisabled() {
+ public FlexibleStringExpander getDisabled() {
Review Comment:
Rename to `getDisabledSpec` to indicate it is no longer returning the
disabled status of a field.
##########
framework/widget/src/main/java/org/apache/ofbiz/widget/model/ModelFormField.java:
##########
@@ -2799,6 +2811,26 @@ public String getValue(Map<String, Object> context) {
return getModelFormField().getEntry(context);
}
+ /**
+ * Gets disabled.
+ * @return the disabled
+ */
+ public FlexibleStringExpander getDisabled() {
+ return disabled;
+ }
+
+ /**
+ *
+ * @param context the context
+ * @return evaluated value
+ */
+ public String getDisabled(Map<String, Object> context) {
Review Comment:
Should return a boolean based on whether the string evaluates to 'true',
'false', or empty (which would also be false).
##########
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:
ok
##########
framework/widget/src/main/java/org/apache/ofbiz/widget/renderer/macro/RenderableFtlFormElementsBuilder.java:
##########
@@ -287,7 +287,7 @@ public RenderableFtl textField(final Map<String, Object>
context, final ModelFor
.stringParameter("id", id)
.stringParameter("event", event != null ? event : "")
.stringParameter("action", action != null ? action : "")
- .booleanParameter("disabled", disabled)
+ .stringParameter("disabled", String.valueOf(disabled))
Review Comment:
I don't think this change is needed. FTL is able to accept boolean
parameters.
One of the ideas behind RenderableFtl's parameters was to avoid encoding
meaning in strings when other more focussed types, such as boolean an int,
would do a better job.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]