FWD » Core Development » User Interface

Greg Shah wrote:

From Marian:

Setting SCREEN-VALUE for a selection list was improved by Adrian as part of #5992, however validation of input value as well as the actual update of the screen-value is not exactly as in 4GL:

• screen-value can be a list (using the list's delimiter), both for single/multi
• validation should validate all entries in the screen-value and fail if any entry not valid, it doesn't matter if single/multi
• when setting a 'single value' only the first item in the list that match must be selected (aka: if list had 's' and 'S' as items only first must be selected - case insensitive)
• when setting a 'multi value' for single selection list the last entry 'win', for multiple selection one item is selected for each entry - previous selection is not clear (append).

Because of the last item in the previous list the value set on SelectionListWidget - although valid - could be different from the actual screen-value of the selection-list. For single select list it will actually be just the last entry in the list, this can be handled in SelectionListWidget. For multi selection list is not that easy since the resulting screen-value could have previously selected items and even the ordering could be different - setting '3,2' as screen-value could result in '2,3' depending on the items order in the list.

Can't you just read the widget's screen value on the server before new one is set and merge it with the new value?

Hynek Cihlar wrote:

Can't you just read the widget's screen value on the server before new one is set and merge it with the new value?

1. GenericFrame.setScreenValue gets called, validate value against data type and pass it to the widget
2. SelectionListWidget.setScreenValue - value validation, put the value in frame's screen-buffer
3. GenericFrame.setScreenValue sends over the new value through LogicalTerminal->ThinClient refreshFrameWidgets methods
4. GenericFrame.setScreenValue set the value as-is in frame's screen-buffer again and reset the status as unchanged

Now, I don't know how to get the new screen-value other than by using the FrameData and adding a return value in refreshFrameWidget. Calling widget's getScreenValue there will simply return the value already set in the frame's screen buffer and even if it would be possible to make a round-trip again to get the updated screen-value why the extra effort when we can send the updated data back directly?

The problem with this approach is that the FrameData seems to be populated with values by calling getText method of widgets which for a SelectionList is only good if not using 'item-pairs'.

Is there a reason for which getText of SelectionList returns the 'display' value instead of the actual item value like screen-value does? I'm not aware of any meaning in 4GL to get the list 'display value', only than by parsing the list-item-pairs but why anyone would do that anyway? :)

Marian Edu wrote:

Hynek Cihlar wrote:

Can't you just read the widget's screen value on the server before new one is set and merge it with the new value?

The workflow as I see it is like this:

1. GenericFrame.setScreenValue gets called, validate value against data type and pass it to the widget
2. SelectionListWidget.setScreenValue - value validation, put the value in frame's screen-buffer
3. GenericFrame.setScreenValue sends over the new value through LogicalTerminal->ThinClient refreshFrameWidgets methods
4. GenericFrame.setScreenValue set the value as-is in frame's screen-buffer again and reset the status as unchanged

Now, I don't know how to get the new screen-value other than by using the FrameData and adding a return value in refreshFrameWidget. Calling widget's getScreenValue there will simply return the value already set in the frame's screen buffer and even if it would be possible to make a round-trip again to get the updated screen-value why the extra effort when we can send the updated data back directly?

The idea is to keep the client a generic and thin presentation logic. All the application logic should happen on the server in the context of the current session. Considering this it seems natural to me that the screen value "merging" for multi-value selection widgets should happen on the server. The screen value is stored on the server anyway so no round trip is necessary when fetching the current widget's value.

Hynek Cihlar wrote:

The idea is to keep the client a generic and thin presentation logic. All the application logic should happen on the server in the context of the current session. Considering this it seems natural to me that the screen value "merging" for multi-value selection widgets should happen on the server. The screen value is stored on the server anyway so no round trip is necessary when fetching the current widget's value.

I see, that sounds like a great idea so will go ahead and implement it there.. it's just that the 'server' side SelectionListWidget it's kinda slim (1k loc) compared to the implementation on the other end so I've got distracted but you're right.

Hynek: Please review the patch and merge it into 3821c if you have no objections.

Marian Edu wrote:

Attached the patch that fixes some issues we've found when setting the screen-value for selection-list. There are some issues with combo box that are left, one interesting quirk in 4GL being that when the screen-value is set to an invalid value that is shown in the combo-box as-is (for simple/drop_down) but when asking for it the 4GL returns the last valid value that was set :(

The patch is good. Just one minor thing. When SLW.setScreenValue assembles the new value it calls the super, which again performs validity of the just-assembled screen value. Please fix this.

Marian Edu wrote:

Hynek Cihlar wrote:

The patch is good. Just one minor thing. When SLW.setScreenValue assembles the new value it calls the super, which again performs validity of the just-assembled screen value. Please fix this.

That did also bug me, can't go around super and call super's super but this will work around the duplicated validation - overriding validation method and if valid update the value right there.

Attached the modified patch, thanks.

The patch is fine. I did a few cosmetic changes to it and checked it in 3821c revision 13469.

I've pushed 3821c rev 13469 to xfer.

What is left open in this task? Please set the % Done.

Greg Shah wrote:

I've pushed 3821c rev 13469 to xfer.

What is left open in this task? Please set the % Done.

For selection list all my tests are passing now, I saw some issues with combo/radio widgets related to set screen-value but it's probably better to make a separate task for that.. unless you think this one should be used instead.

Greg Shah wrote:

A separate task makes sense.

Will do that, however the changes did introduce some regression for combo box widgets that I need to fix.

Hynek, I trust the patch was applied on 'internal' repo, can you please push that into xref so I can make a new incremental patch and also incorporate your 'cosmetic' changes?

Thanks

I've pushed 3821c rev 13469 to xfer.

Marian Edu wrote:

Greg Shah wrote:

I've pushed 3821c rev 13469 to xfer.

Thanks, attached the patch to fix the regression on combo-box - value to be set on screen-value needs not to be split using the delimiter and then validate each entry, this is only true for selection-list.

Marian, does it mean the other affected widgets, BROWSE-COLUMNS, COMBO-BOX and RADIO-SET, can't take multiple values in the SCREEN-VALUE attribute?

Also how does OE/4GL handle escaping of the delimiter character? How does it behave when an item value contains a delimiter character?

Hynek Cihlar wrote:

Marian, does it mean the other affected widgets, BROWSE-COLUMNS, COMBO-BOX and RADIO-SET, can't take multiple values in the SCREEN-VALUE attribute?

Yes, the only widget that accepts multiple values when setting the screen-value is the selection list, all others reports it as invalid.

Also how does OE/4GL handle escaping of the delimiter character? How does it behave when an item value contains a delimiter character?

No escaping done, normally if one uses the AppBuilder (and no-one writes UI in vim) using the delimiter is not possible inside a value at design time. At run-time there is no possibility (at least that I know of) to escape the delimiter so for list-item there will be two entries added instead on one and for list-item-pair an error will be issued because the result will be impair :)

Beside this is the whole idea of being able to change the delimiter, to use one that can't be part of item values (labels).

On the other hand, it is possible to edit the generated code and get around it like this:

DEFINE VARIABLE SELECT-Default AS CHARACTER 
     VIEW-AS SELECTION-LIST MULTIPLE
     LIST-ITEM-PAIRS "default","D",
                     "empty"," ",
                     "second","s",
                     "SECOND","S,D" 
     SIZE 26 BY 2.14 NO-UNDO.

This works, no complain at run-time and get screen-value for SECOND returns S,D, however when you try to set the same value the list will select two items if multi: default and second, or last one if single - so the delimiter is used upfront event if there might be a value with exact match.

The converted code in FWD have the same behaviour.

Marian Edu wrote:

This works, no complain at run-time and get screen-value for SECOND returns S,D, however when you try to set the same value the list will select two items if multi: default and second, or last one if single - so the delimiter is used upfront event if there might be a value with exact match.

The converted code in FWD have the same behaviour.

Sounds good. I checked 6016-20220202.patch in 3821c revision 13476.

Hynek Cihlar wrote:

Sounds good. I checked 6016-20220202.patch in 3821c revision 13476.

Please note that the problem described in #5983-15 still exists with this revision.

Igor Skornyakov wrote:

Please note that the problem described in #5983-15 still exists with this revision.

Yes, this is combo-box widget and there are a few remaining issues with that as well... working on it while I'm here and already have a test made in testcases.

In ControlSetEntity.setScreenValue, we've lost the ability to prevent the error message from being displayed in the change that added ControlSetEntity.validateScreenValue due to this condition being removed for protecting the ErrorManager.recordOrShowWarning:

 && frame != null &&
                getAttr("wasRealized", () -> frame.getFrameWidget().config().wasRealized, true))

My proposed changes:

--- /tmp/brz-diff-g17v03gw/old/src/com/goldencode/p2j/ui/ControlSetEntity.java
+++ /home/rfb/projects/fwd/3821c/src/com/goldencode/p2j/ui/ControlSetEntity.java
@@ -2,7 +2,7 @@
 ** Module   : ControlSetEntity.java
 ** Abstract : server side grouped control widget parent class
 **
-** Copyright (c) 2005-2021, Golden Code Development Corporation.
+** Copyright (c) 2005-2022, Golden Code Development Corporation.
 **
 ** -#- -I- --Date-- --JPRM-- ----------------------------Description-----------------------------
 ** 001 NVS 20051026   @23487 Created initial version.
@@ -97,6 +97,8 @@
 **     AL2 20220124          Make all control set entities to check for screen-value validity.
 **     ME  20220201          Allow for multiple values list when setting screen-value (delimiter is used).
 **     ME  20220202          Do not split value using delimiter on validateScreenValue - only selection-list
+**     RFB 20220204          setScreenValue needs to be able to prevent the error message from being displayed,
+**                           thus allowing a parameter to be passed to validateScreenValue.
 */
 /*
 ** This program is free software: you can redistribute it and/or modify
@@ -2857,7 +2859,8 @@
    {
       if (inUIStmt || !internalScreenValueUsage)
       {
-         if (value instanceof character && !validateScreenValue((character) value))
+         boolean silent = !(frame != null && getAttr("wasRealized", () -> frame.getFrameWidget().config().wasRealized, true));
+         if (value instanceof character && !validateScreenValue((character) value, silent))
          {
             return false;
          }
@@ -2876,9 +2879,25 @@
     */
    boolean validateScreenValue(character value)
    {
+      return validateScreenValue(value, false);
+   }
+
+   /**
+    * Validate the value(s) for SCREEN-VALUE attribute.
+    * 
+    * @param value
+    * The to be validated as SCREEN-VALUE attribute.
+    *
+    * @param silent
+    * Suppress error message when true
+    * 
+    * @return true if the value is a valid screen-value.
+    */
+   boolean validateScreenValue(character value, boolean silent)
+   {
       if (!isValidScreenValue(value))
       {
-         if (_isRealized())
+         if (_isRealized() && !silent)
          {
             ErrorManager.recordOrShowWarning(4058, String.format(
                      "Attribute SCREEN-VALUE for the %s %s has an invalid value of %s", type(),

Or would it be better just to place that condition within validateScreenValue, as it was before?

Roger Borrello wrote:

In ControlSetEntity.setScreenValue, we've lost the ability to prevent the error message from being displayed in the change that added ControlSetEntity.validateScreenValue due to this condition being removed for protecting the ErrorManager.recordOrShowWarning:

My proposed changes are not necessary, due to the updates in #6038 fixing the base issue.

Roger Borrello wrote:

Roger Borrello wrote:

In ControlSetEntity.setScreenValue, we've lost the ability to prevent the error message from being displayed in the change that added ControlSetEntity.validateScreenValue due to this condition being removed for protecting the ErrorManager.recordOrShowWarning:

My proposed changes are not necessary, due to the updates in #6038 fixing the base issue.

Actually the customer indicates there are other combo-boxes in which the error appears (see #6044). Perhaps we should consider the code that was removed?

=== modified file 'src/com/goldencode/p2j/ui/ControlSetEntity.java'
--- old/src/com/goldencode/p2j/ui/ControlSetEntity.java    2022-02-05 16:24:08 +0000
+++ new/src/com/goldencode/p2j/ui/ControlSetEntity.java    2022-02-06 14:01:29 +0000
@@ -2,7 +2,7 @@
 ** Module   : ControlSetEntity.java
 ** Abstract : server side grouped control widget parent class
 **
-** Copyright (c) 2005-2021, Golden Code Development Corporation.
+** Copyright (c) 2005-2022, Golden Code Development Corporation.
 **
 ** -#- -I- --Date-- --JPRM-- ----------------------------Description-----------------------------
 ** 001 NVS 20051026   @23487 Created initial version.
@@ -97,6 +97,8 @@
 **     AL2 20220124          Make all control set entities to check for screen-value validity.
 **     ME  20220201          Allow for multiple values list when setting screen-value (delimiter is used).
 **     ME  20220202          Do not split value using delimiter on validateScreenValue - only selection-list
+**     RFB 20220204          setScreenValue needs to be able to prevent the error message from being displayed,
+**                           thus allowing a parameter to be passed to validateScreenValue.
 */
 /*
 ** This program is free software: you can redistribute it and/or modify
@@ -2886,7 +2888,8 @@
    {
       if (!isValidScreenValue(value))
       {
-         if (_isRealized())
+         if (_isRealized() && (frame != null) &&
+             getAttr("wasRealized", () -> frame.getFrameWidget().config().wasRealized, true))
          {
             ErrorManager.recordOrShowWarning(4058, String.format(
                      "Attribute SCREEN-VALUE for the %s %s has an invalid value of %s", type(),