FWD » Core Development » User Interface

There is C:\Windows\SysWOW64\mscomct2.ocx OCX library that exposes IDTPicker (called Microsoft Date and Time Picker control). Is this one that must be replaced?

Sergey Ivanovskiy wrote:

There is C:\Windows\SysWOW64\mscomct2.ocx OCX library that exposes IDTPicker (called Microsoft Date and Time Picker control). Is this one that must be replaced?

Yes, but it will appear as different names and in different paths depending on the application. To start, I want you to make a list of the functionality that is needed based on reviewing the usage in multiple customer applications. One use case is the OCX but there are other cases where the 4GL code implements its own calendar using existing 4GL widgets (usually buttons, rectangles, images, combo-boxes). Most often this "do it yourself" calendar is written as a separate procedure that uses a dialog box and is just executed in a RUN statement from a button trigger. This button has a little calendar icon and will pop up the dialog and write back the picked date as an input-output parameter.

The second case will need some 4GL code editing, but we first must have our own built-in widget that is a suitable replacement.

I will send a separate email with the list of customer applications to check. I cannot post them here because this is a public task.

In Hotel GUI, you can see this usage as follows. You will see a button widget in some screen or dialog which as a trigger like this:

ON CHOOSE OF startDatePicker IN FRAME gDialog
DO:
    DEF VAR outDate AS DATE INIT ?.
  RUN adatesd0.w (INPUT "Check-in Date", 
                  INPUT startDate:INPUT-VALUE, 
                  OUTPUT outDate).
  IF outDate <> ? THEN DO:
     startDate = outDate.
     DISPLAY startDate WITH FRAME gDialog.     
  END. 
  RUN refresh-days-price.
  RUN refresh-total-price.
END.

The program exists in abl/common/adatesd0.w. It was originally designed as a "smart data object", but I don't think that ADE SDO non-sense is still there. The 4GL code itself is not too hard, but we want to get rid of it.

For details on using MS Common Controls OCXes in appbuilder, please see #3858-85 (through note 87).

A key point: OCX controls are only 32-bit in nature. It is my understanding that Microsoft has never provided a 64-bit development environment or APIs for creating 64-bit OCX controls. This naturally causes issues when using the newer 64-bit OpenEdge development tools.

The following are useful notes about using appbuilder for OCX development, copied from emails between Marius and Sergey):

1. Installing "Visual Studio 2017 Community" will also install a design time license for MS common controls. Those OCX's require a license to be used at design time. There is only a 32-bit version of MS common controls OCX's.

1. The appbuilder in 64-bit mode cannot be used for OCX development, OCX's are not shown in the list. If you do not have a shortcut, it can be started using C:\Progress\OE116_64\bin\prowin.exe -p _ab.p (the path will depend on where your 64-bit OE is installed).

2. It is possible to run the procedure editor from an 64-bit OE version. It would look like C:\Progress\OE116_64\bin\prowin.exe -basekey "INI" -ininame progress11.ini -pf myprofile.pf -p _edit.p. The path, ini and profile may be different for other installations. Interestingly, one can launch the application builder from inside the procedure editor.

3. Based on this technique of launching the 64-bit procedure editor, one could create a shortcut named "Editor32" (name is not important, could be any name) with C:\Progress\OE116_32\bin\prowin32.exe -basekey "INI" -ininame progress11.ini -pf myprofile.pf -p _edit.p.

4. Then, after starting the "Editor32" and Procedure Editor window will open on the screen. From the menu you can start the AppBuilder which will be in 32-bit mode if started from the 32-bit procedure editor.

5. Using the 32-bit version (OE116_32) of the appbuilder, you can inspect 32-bit OCX's which cannot be inspected with OE116_64.

1. AppBuilder creates wrx file that contains the AciveX controls used by the corresponding application file to create UI
   and creates CONTROL-FRAME widget (looks like ActiveX proxy or widget wrapper) per one control instance
2. Get the control handle
   

       comHandle = ControlFrameWidget:COM-HANDLE
   

3. Load controls from wrx files
   

       comHandle:LoadControls( WrxFilePath, ControlFrameWidgetName)
   

4. Get and set these controls properties
   

       comHandle:DateTimePickerControlName:VISIBLE
       comHandle:DateTimePickerControlName:ENABLED
       comHandle:DateTimePickerControlName:VALUE
       comHandle:DateTimePickerControlName:FONT:SIZE
   

I think we only need to worry about item 4. Assuming no design time properties are set via the wrx (see #4438), items 1 through 3 are already supported. Of course, we must create the hints file to enable this support but that is pretty straightforward.

Greg Shah wrote:

I think we only need to worry about item 4. Assuming no design time properties are set via the wrx (see #4438), items 1 through 3 are already supported. Of course, we must create the hints file to enable this support but that is pretty straightforward.

At least the MS Date Time Picker control name is stored in this file. All customer usages don't change the default name for this control that is DTPicker.
But there is another way to access the control via collections

/* Define the widget handle for the window                              */
DEFINE VAR C-Win AS WIDGET-HANDLE NO-UNDO.

/* Definitions of handles for OCX Containers                            */
DEFINE VARIABLE CtrlFrame AS WIDGET-HANDLE NO-UNDO.
DEFINE VARIABLE chCtrlFrame AS COMPONENT-HANDLE NO-UNDO.
DEFINE VARIABLE chCollection AS COM-HANDLE NO-UNDO.
DEFINE VARIABLE chDTPicker AS COM-HANDLE NO-UNDO.

  ASSIGN
    chCtrlFrame = CtrlFrame:COM-HANDLE
    UIB_S = chCtrlFrame:LoadControls( OCXFile, "CtrlFrame":U)
    CtrlFrame:NAME = "CtrlFrame":U.
..........................................................
  chCollection = chCtrlFrame:controls.
  chDTPicker = chCollection:Item(1).
  MESSAGE chDTPicker:VALUE.

Greg Shah wrote:

I think we only need to worry about item 4. Assuming no design time properties are set via the wrx (see #4438), items 1 through 3 are already supported. Of course, we must create the hints file to enable this support but that is pretty straightforward.

Please clarify the hints file conception. It seems that these changes have not been committed into the trunc yet, have they?

Taking into account this working 4GL code #4174-12 the current implementation of ControlFrameComObjectImpl.this.getControls and comhandle (3809e) looks incorrect. It seems that comhandle should implement collection interface to get an item by its index.

Sergey Ivanovskiy wrote:

Greg Shah wrote:

I think we only need to worry about item 4. Assuming no design time properties are set via the wrx (see #4438), items 1 through 3 are already supported. Of course, we must create the hints file to enable this support but that is pretty straightforward.

At least the MS Date Time Picker control name is stored in this file. All customer usages don't change the default name for this control that is DTPicker.
But there is another way to access the control via collections
[...]

Please read that: https://proj.goldencode.com/issues/3856#note-38

At least the MS Date Time Picker control name is stored in this file.

For now, this is put in the hints file in FWD.

Please clarify the hints file conception. It seems that these changes have not been committed into the trunc yet, have they?

It is there. Search for "ext-hints" in rules/. You can also see examples of how to use it in:

Progress Indicator
ImageList
HTMLBrowser
TAB-SET
TREEVIEW

We do have many improvements to the COM support in 3856b (which will be merged to trunk soon). You should build upon those changes but do it in another branch.

Sergey Ivanovskiy wrote:

Taking into account this working 4GL code #4174-12 the current implementation of ControlFrameComObjectImpl.this.getControls and comhandle (3809e) looks incorrect. It seems that comhandle should implement collection interface to get an item by its index.

In fact, only two methods of comhandle is enough to cover all the need:

   public BaseDataType call(String methodName, Object... params)

and

   public comhandle chainCall(String methodName, Object... params)

The only difference between these two methods is how they treat the "missing value" situation: the first method returns the new unknown(), and the second returns new comhandle().

Also, we shall make no difference (and we never need to) between the "COM method" call and "COM property" get/set, since we cannot deduce it from the 4gl source only.

I created the task branch 4174a. Should it be rebased over 3856b? Would it be better to wait until 3856b merged in the trunc?

Please help with these common questions about testing environment. I am still using old testcases project. Is it correct? Should I create a test environment from skeleton project (bzr co ~/secure/code/p2j_repo/skeleton)? I found that for my testcases environment the 4GL code under this macros

&IF "{&OPSYS}" = "WIN32":U AND "{&WINDOW-SYSTEM}" NE "TTY":U &THEN

      <node class="container" name="standard">
        <node class="container" name="runtime">
          <node class="container" name="default">
            <node class="container" name="legacy-system">
              <node class="string" name="pkgroot">
                <node-attribute name="value" value="com.goldencode.testcases"/>
              </node>
              <node class="string" name="opsys">
                <node-attribute name="value" value="WIN32"/>
              </node>
              <node class="string" name="propath">
                <node-attribute name="value" value=".;.\icons\"/>
              </node>
              <node class="string" name="path-separator">
                <node-attribute name="value" value=";"/>
              </node>
              <node class="string" name="file-separator">
                <node-attribute name="value" value="\"/>
              </node>
              <node class="boolean" name="case-sensitive">
                <node-attribute name="value" value="FALSE"/>
              </node>
...................................

I created the task branch 4174a. Should it be rebased over 3856b? Would it be better to wait until 3856b merged in the trunc?

Yes, rebase it from 3856b.

Also: please note that all methods and properties should be exposed as regular (handle-based) 4GL methods and attributes. The objective is to enable "normal 4GL" widget usage for the calendar. This means putting it in a 4GL frame (not a control-frame) and interacting without any of the COM support.

Don't mind this question about &IF "{&OPSYS}" = "WIN32":U AND "{&WINDOW-SYSTEM}" NE "TTY":U &THEN. This macros works as expected.

I am still using old testcases project. Is it correct?

Yes, this is OK.

Should I create a test environment from skeleton project (bzr co ~/secure/code/p2j_repo/skeleton)?

No. The skeletons project is for OO built-in classes and has nothing to do with a testcases project.

was excluded even if OPSYS is set to WIN32 in directory.xml

The directory is used at runtime with the server. It is ignored completely during conversion. Such configuration needs to be in the p2j.cfg.xml, right?

Sergey Ivanovskiy wrote:

Please help with these common questions about testing environment. I am still using old testcases project...

I have created a templates-based FWD project generator once. You can try it also if you want.

To create a new project you have to checkout this skeleton project from bzr repository, run the configuration script and answer a number of questions, the reasonable default values are provided for most of them.

Along with ant scripts, the project created also supports eclipse and maven in case you need it.

Currently this project is not published in GCD repos, I will be glad to do it in case anyone needs it.

We should look at your new project code for sure. Please note that we currently create new projects from the Hotel ChUI and Hotel GUI templates. So there are already some other ways to do this. I have other ideas for a long term solution.

Anyway, there is an advantage to using the existing testcases project: everyone on the team can access it. If we create separate projects for every task, then it will be very difficult to manage over time.

Greg Shah wrote:

The directory is used at runtime with the server. It is ignored completely during conversion. Such configuration needs to be in the p2j.cfg.xml, right?

Yes, thank you. It works with <parameter name="opsys" value="WIN32" /> in p2j.cfg.xml.

Vladimir Tsichevski wrote:

Sergey Ivanovskiy wrote:

Please help with these common questions about testing environment. I am still using old testcases project...

I have created a templates-based FWD project generator once. You can try it also if you want.

Thank you. Please share your script.

Greg Shah wrote:

We should look at your new project code for sure. Please note that we currently create new projects from the Hotel ChUI and Hotel GUI templates. So there are already some other ways to do this. I have other ideas for a long term solution.

I used the Hotel GUI project as the starting point.

Anyway, there is an advantage to using the existing testcases project: everyone on the team can access it.

The project generator just creates a project to work on some issue. It does not replace the testcases project, the later is a collection of thousands of scripts accumulated since the beginning of time which I do not need while I am working on a particular issue.

If we create separate projects for every task, then it will be very difficult to manage over time.

You will probably never need it.

My usual workflow is as follows:

1. I am assigned a new task number #1234
2. I create a FWD core branch named 1234a in central repository
3. I checkout the 1234a to my workstation
4. I checkout my fwdtemplate project somewhere at my workstation at the name test1234a
5. I run

cd test1234a
./prepare_template.sh 

6. and answer questions. The only required answer is the location of the FWD code project to use for run and conversion.

Now I've got a fully functioning "Hello" project.

7. I build and run the project it with:

ant clean convert compile jar
ant server
and client

A message box is shown with the "Hello, world" message.

8. I do my work on the issue, making changes in the 1234a project
9. when I'm done, I archive and/or delete the test123a project. In most cases the project has no valuable stuff to keep.

So you've made changes to the prepare_template.sh and build.xml?

Greg Shah wrote:

So you've made changes to the prepare_template.sh and build.xml?

I have rewritten the prepare_template.sh. Most of other files, build.xml included, are generated automatically from 17 template files.

Are the prepare_template.sh changes something generic enough to be accepted back as a standard part of the Hotel GUI project?

Greg Shah wrote:

Are the prepare_template.sh changes something generic enough to be accepted back as a standard part of the Hotel GUI project?

I think, the Hotel GUI project can be created from the fwdtemplate, if it is what you mean. But this need to be checked.

I think, the Hotel GUI project can be created from the fwdtemplate, if it is what you mean. But this need to be checked.

We have a bzr project that contains the Hotel GUI project. We update that with changes over time as needed. It includes everything needed to convert and run Hotel GUI. It also contains the original prepare_template.sh which you have modified. We release the latest version of this project to the public when we release a new public version of FWD. This is what most people use to make a new project or to test FWD.

Is "fwdtemplate", just a version of Hotel GUI that has changes to prepare_template.sh? If so AND if the changes are safe for general usage, then we could check in that changed version to our Hotel GUI bzr project. First we would want to review the changes to understand the implications.

I just tested the conversion of my example with 3856b if ext-hint file is given and found that this code

DEFINE VARIABLE CtrlFrame AS WIDGET-HANDLE NO-UNDO.
DEFINE VARIABLE chCtrlFrame AS COMPONENT-HANDLE NO-UNDO.
DEFINE VARIABLE chDTPicker AS COM-HANDLE NO-UNDO.
.................................
chDTPicker = chCtrlFrame:DTPicker.
strDate = chDTPicker:VALUE.

chDtpicker.assign(hDTPicker.getProperty("DTPicker"));
strDate.assign(chDtpicker.getProperty("VALUE"));

chCtrlFrame:DTPicker:VALUE

<extension>
   <ocx control-frame="CtrlFrame" control-frame-handle="chCtrlFrame" com-handle="chCtrlFrame:DTPicker" target-handle="hDTPicker" target-widget="progress-bar" />
   <drop-procedure name="control_load" />
</extension>

Sorry, it was 3856b. It seems that it looks like an issue in 3856b.

It seems that developing the date time picker widget itself can be independent from the conversion issues.

Greg, are there special requirements for this widget?
Should it be designing exactly the same as the MS Date and Time Picker control? The web client can use the dojo date picker control and the swing client can utilize another swing date picker control, for an example, https://sourceforge.net/projects/jdatepicker/.

Should it be designing exactly the same as the MS Date and Time Picker control?

Yes. This is needed for compatibility with the OCX. We have multiple customers that will need it to work this way.

The web client can use the dojo date picker control and the swing client can utilize another swing date picker control, for an example, https://sourceforge.net/projects/jdatepicker/.

These cannot be used. The secondary (and also important) objective here is to add a new 4GL widget. Using existing widgets from DOJO or Swing does not work with our design.

Sergey Ivanovskiy wrote:

Sorry, it was 3856b. It seems that it looks like an issue in 3856b.

Yes, you have right chCtrlFrame:DTPicker must be always converted into hDTPicker I've fixed that in 3856b rev 11364 to be compliant with the current trunk conversion rules. You can cherry-pick my changes.
Moreover, when such statements are detected a warning message is written into the log.

However this wil not fix the runtime issue. As a result of conversion we assign a handle to a comhandle, chDTPicker.assign(hDTPicker); (see comhandle.java method assign(BaseDataType value)).

The OCX conversion rules are designed to convert OCX com-handle into a 4gl widget handle and this is a tricky operation. That's why currently OCX conversion does not cover all possible scenarios.

One scenario is what you already experienced. Normally when we convert that: chDTPicker = chCtrlFrame:DTPicker. We should also convert definition statement:
  DEFINE VARIABLE chDTPicker AS COM-HANDLE NO-UNDO.
into:
  DEFINE VARIABLE chDTPicker AS HANDLE NO-UNDO.
But this is not possible without additional hints and could be error prone.

Another possible scenario is when OCX handle is used as a parameter in a procedure.

RUN PROC-A chCtrlFrame:DTPicker.

PROCEDURE PROC-A.
   DEFINE VARIABLE chDTPicker AS COM-HANDLE NO-UNDO.
END.

Here we have the same problem because chCtrlFrame:DTPicker is converted into a widget handle and we call PROC-A with a handle instead a com-handle
In order to fix that, we have to change also the DEFINE VARIABLE chDTPicker AS COM-HANDLE NO-UNDO. statement into DEFINE VARIABLE chDTPicker AS HANDLE NO-UNDO.
This is difficult to solve without additional hints. The recommendation is to avoid such scenarios and to use statements like chCtrlFrame:DTPicker:VALUE.

Just now I'm working to test the changes for OCX conversion rules I did in 3856b. Doing conversions on a set of large projects so far I found no such scenarios.

Greg Shah wrote:

Should it be designing exactly the same as the MS Date and Time Picker control?

Yes. This is needed for compatibility with the OCX. We have multiple customers that will need it to work this way.

The web client can use the dojo date picker control and the swing client can utilize another swing date picker control, for an example, https://sourceforge.net/projects/jdatepicker/.

These cannot be used. The secondary (and also important) objective here is to add a new 4GL widget. Using existing widgets from DOJO or Swing does not work with our design.

It would be good to reuse some GUI client classes ComboBoxGuiImpl, DropDownGuiImpl, EntryFieldGuiImpl adding possibility to work with date/time data type. This widget should be able to display the date/time data type and to edit the current value by keys. UP/DOWN keys are used to increase/to decrease the current component's value and LEFT/RIGHT keys are used to move the current focus to the target component. Also it should display the pair of spin buttons if the selected format is time. In this case the calendar popup should not be displayed. I didn't find how to change the drop down button for the MS Date and Time Picker control. For my windows 12 classics theme it uses the arrow button as a classic combobox.

This resource https://wutils.com/com-dll/constants/constants-MSComCtl2.htm defines related IDTPicker constants from mscomct2.ocx.

It seems that IFontDisp should be added but there exists the similar interface CustomFont. The CustomFont uses FONT-NAME and FONT-SIZE legacy names, but IFontDisp uses NAME and SIZE respectively. The customer application uses setter for SIZE property via comHandle:DateTimePickerControlName:FONT:SIZE. The control's font point size can be decimal value as in this test: MS Shell Dlg 8.25 point size.

I tried to add conversion rules for FONT:NAME and FONT:SIZE legacy attributes of ActiveX control using these rules

         <!-- DTPicker -->
         <action>map6 = create("java.util.HashMap")</action>
         <action>map6.put("value"             , "DTPValue")</action>
         <action>map6.put("CustomFormat"             , "CustomFormat")</action>
         <action>map6.put("font:name"             , "ActiveXFontName")</action>
         <action>map6.put("font:size"             , "ActiveXFontSize")</action>
         <action>aliasMap.put("DATETIME-PICKER"   , map6)</action>

  currDate = DATE(chCtrlFrame:DTPicker:VALUE).
  chCtrlFrame-4:DTPicker:CustomFormat = "yyyy-MM-dd (hh:mm)".
  message chCtrlFrame-4:DTPicker:VALUE "CustomFormat=" chCtrlFrame-4:DTPicker:CustomFormat " font(handle): " chCtrlFrame-4:DTPicker:FONT chCtrlFrame-4:DTPicker:FONT:NAME chCtrlFrame-4:DTPicker:FONT:SIZE.

         currDate.assign(new date(hDTPicker.unwrapDateTimePicker().getDTPValue()));
         hDTPicker4.unwrapDateTimePicker().setCustomFormat(new character("yyyy-MM-dd (hh:mm)"));
         message(new Object[]
         {
            hDTPicker4.unwrapDateTimePicker().getDTPValue(),
            "CustomFormat=",
            hDTPicker4.unwrapDateTimePicker().getCustomFormat(),
            " font(handle): ",
            hDTPicker4.unwrapWidget().getFont(),
            hDTPicker4.unwrapWidget().getFont().getProperty("NAME"),
            hDTPicker4.unwrapWidget().getFont().getProperty("SIZE")
         });

chCtrlFrame-4:DTPicker:FONT:NAME chCtrlFrame-4:DTPicker:FONT:SIZE

            hDTPicker4.unwrapWidget().getFont().getProperty("NAME"),
            hDTPicker4.unwrapWidget().getFont().getProperty("SIZE")

Sergey Ivanovskiy wrote:

I tried to add conversion rules for FONT:NAME and FONT:SIZE legacy attributes of ActiveX control using these rules
[...]
but this code fragment
[...]
is converted into
[...]
The CustomFormat legacy attribute is converted properly, because it has been added as new 4GL key words, but these ones
[...]
are converted into
[...]
Hynek, Marius do you know where can be these rules that control such conversions. We have getFont() and probably chCtrlFrame-4:DTPicker:FONT is mapped to widget custom font handle.

This should be fixed in 3856b. Please rebase your branch with 3856b in order to get last changes.

Marius, thanks, I updated 4174b over 3856a and got this AST node for chCtrlFrame-4:DTPicker:FONT:SIZE expression

message chCtrlFrame-4:DTPicker:VALUE "CustomFormat=" chCtrlFrame-4:DTPicker:CustomFormat " font(handle): " chCtrlFrame-4:DTPicker:FONT chCtrlFrame-4:DTPicker:FONT:NAME chCtrlFrame-4:DTPicker:FONT:SIZE.

        <ast col="0" id="12884903530" line="0" text="statement" type="STATEMENT">
          <ast col="3" id="12884903531" line="360" text="message" type="KW_MSG">
            <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
            <ast col="0" id="12884903533" line="0" text="" type="CONTENT_ARRAY">
              <ast col="0" id="12884903534" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <ast col="33" id="12884903535" line="360" text=":" type="COM_INVOCATION">
                  <ast col="24" id="12884903536" line="360" text=":" type="COM_INVOCATION">
                    <ast col="11" id="12884903537" line="360" text="chCtrlFrame-4" type="VAR_COM_HANDLE">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                      <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                      <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                    </ast>
                    <ast col="25" id="12884903538" line="360" text="DTPicker" type="COM_PROPERTY">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                    </ast>
                  </ast>
                  <ast col="34" id="12884903539" line="360" text="VALUE" type="COM_PROPERTY">
                    <annotation datatype="java.lang.Long" key="oldtype" value="907"/>
                  </ast>
                </ast>
              </ast>
              <ast col="0" id="12884903541" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <ast col="40" id="12884903542" line="360" text="&quot;CustomFormat=&quot;" type="STRING">
                  <annotation datatype="java.lang.Boolean" key="is-literal" value="true"/>
                  <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                </ast>
              </ast>
              <ast col="0" id="12884903544" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <ast col="78" id="12884903545" line="360" text=":" type="COM_INVOCATION">
                  <ast col="69" id="12884903546" line="360" text=":" type="COM_INVOCATION">
                    <ast col="56" id="12884903547" line="360" text="chCtrlFrame-4" type="VAR_COM_HANDLE">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                      <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                      <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                    </ast>
                    <ast col="70" id="12884903548" line="360" text="DTPicker" type="COM_PROPERTY">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                    </ast>
                  </ast>
                  <ast col="79" id="12884903549" line="360" text="CustomFormat" type="COM_PROPERTY">
                    <annotation datatype="java.lang.Long" key="oldtype" value="2773"/>
                  </ast>
                </ast>
              </ast>
              <ast col="0" id="12884903551" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <ast col="92" id="12884903552" line="360" text="&quot; font(handle): &quot;" type="STRING">
                  <annotation datatype="java.lang.Boolean" key="is-literal" value="true"/>
                  <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                </ast>
              </ast>
              <ast col="0" id="12884903554" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <ast col="132" id="12884903555" line="360" text=":" type="COM_INVOCATION">
                  <ast col="123" id="12884903556" line="360" text=":" type="COM_INVOCATION">
                    <ast col="110" id="12884903557" line="360" text="chCtrlFrame-4" type="VAR_COM_HANDLE">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                      <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                      <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                    </ast>
                    <ast col="124" id="12884903558" line="360" text="DTPicker" type="COM_PROPERTY">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                    </ast>
                  </ast>
                  <ast col="133" id="12884903559" line="360" text="FONT" type="COM_PROPERTY">
                    <annotation datatype="java.lang.Long" key="oldtype" value="645"/>
                  </ast>
                </ast>
              </ast>
              <ast col="0" id="12884903561" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <ast col="165" id="12884903562" line="360" text=":" type="COM_INVOCATION">
                  <ast col="160" id="12884903563" line="360" text=":" type="COM_INVOCATION">
                    <ast col="151" id="12884903564" line="360" text=":" type="COM_INVOCATION">
                      <ast col="138" id="12884903565" line="360" text="chCtrlFrame-4" type="VAR_COM_HANDLE">
                        <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                        <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                        <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                      </ast>
                      <ast col="152" id="12884903566" line="360" text="DTPicker" type="COM_PROPERTY">
                        <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                      </ast>
                    </ast>
                    <ast col="161" id="12884903567" line="360" text="FONT" type="COM_PROPERTY">
                      <annotation datatype="java.lang.Long" key="oldtype" value="645"/>
                    </ast>
                  </ast>
                  <ast col="166" id="12884903568" line="360" text="NAME" type="COM_PROPERTY">
                    <annotation datatype="java.lang.Long" key="oldtype" value="1887"/>
                  </ast>
                </ast>
              </ast>
              <ast col="0" id="12884903570" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <ast col="198" id="12884903571" line="360" text=":" type="COM_INVOCATION">
                  <ast col="193" id="12884903572" line="360" text=":" type="COM_INVOCATION">
                    <ast col="184" id="12884903573" line="360" text=":" type="COM_INVOCATION">
                      <ast col="171" id="12884903574" line="360" text="chCtrlFrame-4" type="VAR_COM_HANDLE">
                        <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                        <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                        <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                      </ast>
                      <ast col="185" id="12884903575" line="360" text="DTPicker" type="COM_PROPERTY">
                        <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                      </ast>
                    </ast>
                    <ast col="194" id="12884903576" line="360" text="FONT" type="COM_PROPERTY">
                      <annotation datatype="java.lang.Long" key="oldtype" value="645"/>
                    </ast>
                  </ast>
                  <ast col="199" id="12884903577" line="360" text="SIZE" type="COM_PROPERTY">
                    <annotation datatype="java.lang.Long" key="oldtype" value="2311"/>
                  </ast>
                </ast>
              </ast>
            </ast>
          </ast>
        </ast>

        <ast col="0" id="12884903530" line="0" text="statement" type="STATEMENT">
          <ast col="3" id="12884903531" line="360" text="message" type="KW_MSG">
            <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
            <annotation datatype="java.lang.Long" key="peerid" value="94489281326"/>
            <ast col="0" id="12884903533" line="0" text="" type="CONTENT_ARRAY">
              <annotation datatype="java.lang.Long" key="peerid" value="94489281329"/>
              <ast col="0" id="12884903534" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <annotation datatype="java.lang.Long" key="peerid" value="94489281330"/>
                <ast col="33" id="12884903535" line="360" text=":" type="COLON">
                  <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                  <annotation datatype="java.lang.Long" key="peerid" value="94489281331"/>
                  <ast col="11" id="12884903537" line="360" text="hDTPicker4" type="VAR_HANDLE">
                    <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                    <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                    <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                    <annotation datatype="java.lang.Long" key="peerid" value="94489281333"/>
                  </ast>
                  <ast col="34" id="12884903539" line="360" text="DTP-VALUE" type="ATTR_DATE">
                    <annotation datatype="java.lang.Long" key="oldtype" value="2772"/>
                  </ast>
                </ast>
              </ast>
              <ast col="0" id="12884903541" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <annotation datatype="java.lang.Long" key="peerid" value="94489281334"/>
                <ast col="40" id="12884903542" line="360" text="&quot;CustomFormat=&quot;" type="STRING">
                  <annotation datatype="java.lang.Boolean" key="is-literal" value="true"/>
                  <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                  <annotation datatype="java.lang.Long" key="peerid" value="94489281335"/>
                </ast>
              </ast>
              <ast col="0" id="12884903544" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <annotation datatype="java.lang.Long" key="peerid" value="94489281336"/>
                <ast col="78" id="12884903545" line="360" text=":" type="COLON">
                  <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                  <annotation datatype="java.lang.Long" key="peerid" value="94489281337"/>
                  <ast col="56" id="12884903547" line="360" text="hDTPicker4" type="VAR_HANDLE">
                    <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                    <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                    <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                    <annotation datatype="java.lang.Long" key="peerid" value="94489281339"/>
                  </ast>
                  <ast col="79" id="12884903549" line="360" text="CustomFormat" type="ATTR_CHAR">
                    <annotation datatype="java.lang.Long" key="oldtype" value="2773"/>
                  </ast>
                </ast>
              </ast>
              <ast col="0" id="12884903551" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <annotation datatype="java.lang.Long" key="peerid" value="94489281340"/>
                <ast col="92" id="12884903552" line="360" text="&quot; font(handle): &quot;" type="STRING">
                  <annotation datatype="java.lang.Boolean" key="is-literal" value="true"/>
                  <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                  <annotation datatype="java.lang.Long" key="peerid" value="94489281341"/>
                </ast>
              </ast>
              <ast col="0" id="12884903554" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <annotation datatype="java.lang.Long" key="peerid" value="94489281342"/>
                <ast col="132" id="12884903555" line="360" text=":" type="COLON">
                  <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                  <annotation datatype="java.lang.Long" key="peerid" value="94489281343"/>
                  <ast col="110" id="12884903557" line="360" text="hDTPicker4" type="VAR_HANDLE">
                    <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                    <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                    <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                    <annotation datatype="java.lang.Long" key="peerid" value="94489281345"/>
                  </ast>
                  <ast col="133" id="12884903559" line="360" text="ActiveXFont" type="ATTR_HANDLE">
                    <annotation datatype="java.lang.Long" key="oldtype" value="2774"/>
                  </ast>
                </ast>
              </ast>
              <ast col="0" id="12884903561" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <annotation datatype="java.lang.Long" key="peerid" value="94489281346"/>
                <ast col="165" id="12884903562" line="360" text=":" type="COM_INVOCATION">
                  <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                  <annotation datatype="java.lang.Long" key="peerid" value="94489281347"/>
                  <ast col="160" id="12884903563" line="360" text=":" type="COLON">
                    <annotation datatype="java.lang.Long" key="peerid" value="94489281348"/>
                    <ast col="138" id="12884903565" line="360" text="hDTPicker4" type="VAR_HANDLE">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                      <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                      <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                      <annotation datatype="java.lang.Long" key="peerid" value="94489281350"/>
                    </ast>
                    <ast col="161" id="12884903567" line="360" text="ActiveXFont" type="ATTR_HANDLE">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2774"/>
                    </ast>
                  </ast>
                  <ast col="166" id="12884903568" line="360" text="NAME" type="COM_PROPERTY">
                    <annotation datatype="java.lang.Long" key="oldtype" value="1887"/>
                    <annotation datatype="java.lang.Boolean" key="com-call" value="true"/>
                    <annotation datatype="java.lang.Long" key="peerid" value="94489281351"/>
                  </ast>
                </ast>
              </ast>
              <ast col="0" id="12884903570" line="0" text="expression" type="EXPRESSION">
                <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                <annotation datatype="java.lang.Long" key="peerid" value="94489281352"/>
                <ast col="198" id="12884903571" line="360" text=":" type="COM_INVOCATION">
                  <annotation datatype="java.lang.Boolean" key="wrap" value="true"/>
                  <annotation datatype="java.lang.Long" key="peerid" value="94489281353"/>
                  <ast col="193" id="12884903572" line="360" text=":" type="COLON">
                    <annotation datatype="java.lang.Long" key="peerid" value="94489281354"/>
                    <ast col="171" id="12884903574" line="360" text="hDTPicker4" type="VAR_HANDLE">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2834"/>
                      <annotation datatype="java.lang.Long" key="refid" value="12884902057"/>
                      <annotation datatype="java.lang.Long" key="support_level" value="16400"/>
                      <annotation datatype="java.lang.Long" key="peerid" value="94489281356"/>
                    </ast>
                    <ast col="194" id="12884903576" line="360" text="ActiveXFont" type="ATTR_HANDLE">
                      <annotation datatype="java.lang.Long" key="oldtype" value="2774"/>
                    </ast>
                  </ast>
                  <ast col="199" id="12884903577" line="360" text="SIZE" type="COM_PROPERTY">
                    <annotation datatype="java.lang.Long" key="oldtype" value="2311"/>
                    <annotation datatype="java.lang.Boolean" key="com-call" value="true"/>
                    <annotation datatype="java.lang.Long" key="peerid" value="94489281357"/>
                  </ast>
                </ast>
              </ast>
            </ast>
          </ast>
        </ast>

         <!-- DTPicker -->
         <action>map6 = create("java.util.HashMap")</action>
         <action>map6.put("value"             , "DTPValue")</action>
         <action>map6.put("CustomFormat"             , "CustomFormat")</action>
         <action>map6.put("font"             , "ActiveXFont")</action>
         <action>map6.put("font:name"             , "ActiveXFontName")</action>
         <action>map6.put("font:size"             , "ActiveXFontSize")</action>
         <action>aliasMap.put("DATETIME-PICKER"   , map6)</action>

Sergey Ivanovskiy wrote:

Marius, thanks, I updated 4174a over 3856a and got this AST node for chCtrlFrame-4:DTPicker:FONT:SIZE expression
[...]
with F2 front end mode on
[...]
and after applying ocx_conversion.rules
[...]
Thus we still have hDTPicker4.unwrapIFontDisp().getActiveXFont().getProperty("SIZE") instead of hDTPicker4.unwrapIFontDisp().getActiveXFont().getActiveXFontSize()
I used these map rules
[...]
Working on this issue now.

Sergey, you should rebase on top of 3856b not 3856a. Branch 3856a is no longer used.

Marius Gligor wrote:

Sergey Ivanovskiy wrote:

Marius, thanks, I updated 4174a over 3856a and got this AST node for chCtrlFrame-4:DTPicker:FONT:SIZE expression
[...]
with F2 front end mode on
[...]
and after applying ocx_conversion.rules
[...]
Thus we still have hDTPicker4.unwrapIFontDisp().getActiveXFont().getProperty("SIZE") instead of hDTPicker4.unwrapIFontDisp().getActiveXFont().getActiveXFontSize()
I used these map rules
[...]
Working on this issue now.

Sergey, you should rebase on top of 3856b not 3856a. Branch 3856a is no longer used.

I followed you and rebased over 3856b. This note has an erroneous "3856a" and "3856b" must be in this place. I will fix #4174-45 now.

Sergey, the problem is that you have the following item in map <action>map6.put("font" , "ActiveXFont")</action>

<action>map6.put("font"             , "ActiveXFont")</action>
<action>map6.put("font:name"        , "ActiveXFontName")</action>
<action>map6.put("font:size"        , "ActiveXFontSize")</action>

Conversion rules will find first font and resolve it, that's why generate what you already mentioned.
Just delete this line and conversion should works. Also make sure you don't have any widget method or property font implemented on server side.
When you use font:name or font:size means a composed property and all methods and properties of font should be composed.
If you want to convert font as well you should modify the conversion rule.

Yes, I would like to convert FONT to ActiveXFont, FONT:NAME to ActiveXFont:ActiveXFontName, FONT:SIZE to ActiveXFont:ActiveXFontSize. The FONT of ActiveX component is represented by IFontDisp and can be used by all ActiveX components that must work with fonts.

Another issue can be observed with this expression chCtrlFrame-4:DTPicker:VISIBLE and chCtrlFrame-4:DTPicker:ENABLED converted as hDTPicker4.unwrapVisible().isVisible() and hDTPicker4.unwrapEnabled().isEnabled(). Some unwrap conversion rules are applied incorrectly.

Sergey Ivanovskiy wrote:

Another issue can be observed with this expression chCtrlFrame-4:DTPicker:VISIBLE and chCtrlFrame-4:DTPicker:ENABLED converted as hDTPicker4.unwrapVisible().isVisible() and hDTPicker4.unwrapEnabled().isEnabled(). Some unwrap conversion rules are applied incorrectly.

I am using new common interfaces and their mixes: IFontDisp, IActiveXComponent, IActiveXUIComponent in this DateTimePicker, DateTimePickerWidget, DateTimePickerConfig and DateTimePickerGuiImpl component development. Also these new inerfaces can be used by already implemented and future components that mimic MS ActiveX components. This way is quite straightforward and powerful.

Now I reused the conversion rules of ocx_conversion.rules (version 3856b rev 11371) to convert chCtrlFrame-4:DTPicker:VISIBLE, chCtrlFrame-4:DTPicker:ENABLED,
chCtrlFrame-4:DTPicker:VALUE, chCtrlFrame-4:DTPicker:CustomFormat, chCtrlFrame-4:DTPicker:FONT, chCtrlFrame-4:DTPicker:FONT:NAME and chCtrlFrame-4:DTPicker:FONT:SIZE to hDTPicker4.unwrapIActiveXUIComponent().isOcxVisible() (setOcxVisible(.)), hDTPicker4.unwrapIActiveXComponent().isOcxEnabled() (setOcxEnabled(.)), hDTPicker4.unwrapDateTimePicker().getDateTimeValue()(setDateTimeValue), hDTPicker4.unwrapDateTimePicker().getCustomFormat()(setCustomFormat), hDTPicker4.unwrapIFontDisp().getActiveXFont() (setActiveXFont()), hDTPicker4.unwrapIFontDisp().getActiveXFontName()(setActiveXFontName), hDTPicker4.unwrapIFontDisp().getActiveXFontSize()(setActiveXFontSize()) respectively.

The date and time control should display its data using the current locale settings. For the web client it is the current web client settings and it needs special efforts to get the client locale settings or the client date and time settings. The date and time on the web client side and on the jscript client side can be different. It seems that Today date can be displayed incorrectly on the jscript client side. It needs to synchronize the web client and the jscript client today date and time, isn't it?

For the near future, all date/time processing will be done in the Java code. We are currently ignoring the JS locale. Eventually, perhaps we will change this, but for now it is in Java only.

Can this case confuse users if the today date will be different from the current jscript client date?

It will be consistent with the rest of the application. You aren't writing a javascript widget, right? You are writing a java widget and all the other java code will be consistent with your approach. There is no aspect of the solution that uses JS locales currently. To use them will be out of place.

Yes. The date and time picker extends AbstractWidget, even it is similar to ComboBoxGuiImpl, but it has another presentation and edit functionality.

public class DateTimePickerGuiImpl
extends AbstractWidget<GuiOutputManager>
implements WidgetWithConfig<DateTimePickerConfig>,
           FocusListener

Now I understand that for an example, the implementation of 4GL NOW will be consistent with this widget too.

Correct.

The branch 4174a has been branched from the trunc. The committed rev 11342 added the conversion support for the date and time picker (part 1). Please review ocx_conversion.rules. Working on the part 2 and 3.

Rebased 4174a over the current trunc rev 11342. Committed 11345 (4174a) with the fix for GuiDriver.drawStringWithinBox. Working on the date time picker popup component.

The original MS Date and Time Picker can parse values given as a string in the ISO-8601 calendar system and as a string given by its custom format or its local format given by its style format constants. For an example,

chCtrlFrame-4:DTPicker:Format = 0; /* FULL format style like "Saturday, February 1, 2020" */
chCtrlFrame-4:DTPicker:VALUE = "2020-02-01". /** ISO-8601 */
chCtrlFrame-4:DTPicker:VALUE = "February 1, 2020". /* accepted this string */
chCtrlFrame-4:DTPicker:VALUE = "Saturday, February 1, 2020". /* throws severe exception*/

Please review rev 11348 (4174a). The rest components to do are the date picker viewport and popup, drop down and updown buttons implementations. I found two examples of using FWD widgets (buttons), AlertBoxCommons and PrintSetupDialog. Planning to use the already presented widgets iff there is no a special requirement that these components should be presented as drawings on the canvas of the date and time picker widget.

There is no special requirement. Reuse of the button is acceptable.

Hynek: Please review.

Code review 4174a against latest trunk. I'm not sure if this review isn't too early, given the number of TODOs and missing docs.

In ocx_conversion.rules, map6 map must have all the keys lower cased.

methods_attributes.rules, the keywords should be sorted alphabetically.

peogress.g, attrsAndMethods entries should be sorted alphabetically.

Please remove the duplicate legacy attributes with the "OCX" prefix, like OCX-ENABLED and OCX-VISIBLE. Instead use the existing legacy interfaces that already declare these attributes. Also rename all the identifiers having the "ocx" substring. The ocx in the name may be misleading as the date picker widget has no connection to the OCX technology (other than that we model it after an existing OCX component).

Please remove the interfaces IActiveXUIComponent and IActiveXComponent. Again, we don't want to confuse the interface with ActiveX - a technology not used in the widget. OCX-ENABLED and OCX-VISIBLE should be replaced with ENABLED and VISIBLE from Widget interface. We already have getHWND() method in the interface CommonWidget. Move all the other methods in the DateTimePicker interface.

What is the purpose of IActiveXUIComponent.setHwnd() method? Is this actually called from the converted legacy code?

Some of the methods in DateTimePicker are missing LegacyAttribute annotation.

DateTimePickerConfig.DateTimePickerConfig() miss javadocs.

Making DateFormat.*_FIELD and ChronoField.* (inited in DateFormatFieldsAdapter) Java enums would make things a bit simpler, you wouldn't need the static arrays to keep all the values and the code to initialize the values and to manage the values. Also you would get type safety.

DateTimePickerWidget.comData needs javadoc.

TODOs in DateTimePickerWidget.

Several DateTimePickerGuiImpl fields and methods need javadoc.

DateTimePickerGuiImpl.draw() needs javadoc and the drawing code should be offloaded to the theme classes.

DateTimePickerGuiImpl.width and height should contain a dimension logic to give correct size of the widget when its size is not assigned in the legacy code.

DateTimePickerGuiImpl.onFocusGained and onFocusLost - are these methods needed. They only call repaint().

Please remove the interface IFontDisp. We already have an interface (and common implementation) to allow custom fonts, CustomFont.

LegacyResource.DATETIME_PICKER should be placed before DIALOG_BOX.

Missing file history entries.

Hynek Cihlar wrote:

Please remove the duplicate legacy attributes with the "OCX" prefix, like OCX-ENABLED and OCX-VISIBLE. Instead use the existing legacy interfaces that already declare these attributes. Also rename all the identifiers having the "ocx" substring. The ocx in the name may be misleading as the date picker widget has no connection to the OCX technology (other than that we model it after an existing OCX component).

Please remove the interfaces IActiveXUIComponent and IActiveXComponent. Again, we don't want to confuse the interface with ActiveX - a technology not used in the widget. OCX-ENABLED and OCX-VISIBLE should be replaced with ENABLED and VISIBLE from Widget interface. We already have getHWND() method in the interface CommonWidget. Move all the other methods in the DateTimePicker interface.
DateTimePickerConfig.DateTimePickerConfig() miss javadocs.

I don't agree with these notes at all. Please investigate the code of ProgressBar. It has these methods

/**
 * Return a value that determines whether a form or control can respond to user-generated events.
 * 
 * @return true if enabled false if not.
    */
   @LegacyMethod(name = "PB-ENABLED")
   public logical isEnabled();

/**
 * Set a value that determines whether a form or control can respond to user-generated events.
 * 
 * @param   value
 *          true if enabled false if not.
    */
   @LegacyMethod(name = "PB-ENABLED")
   public void setEnabled(logical value);

So I introduced these interfaces in order that widgets which have corresponding 4GL ActiveX components can use common key words.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Please remove the duplicate legacy attributes with the "OCX" prefix, like OCX-ENABLED and OCX-VISIBLE. Instead use the existing legacy interfaces that already declare these attributes. Also rename all the identifiers having the "ocx" substring. The ocx in the name may be misleading as the date picker widget has no connection to the OCX technology (other than that we model it after an existing OCX component).

Please remove the interfaces IActiveXUIComponent and IActiveXComponent. Again, we don't want to confuse the interface with ActiveX - a technology not used in the widget. OCX-ENABLED and OCX-VISIBLE should be replaced with ENABLED and VISIBLE from Widget interface. We already have getHWND() method in the interface CommonWidget. Move all the other methods in the DateTimePicker interface.
DateTimePickerConfig.DateTimePickerConfig() miss javadocs.

I don't agree with these notes at all. Please investigate the code of ProgressBar. It has these methods

Which part you don't agree with? The "OCX" confusing the interfaces or the duplicated attributes?

/**

• Return a value that determines whether a form or control can respond to user-generated events.
• @return true if enabled false if not.
  */
  @LegacyMethod(name = "PB-ENABLED")
  public logical isEnabled();

/**

• Set a value that determines whether a form or control can respond to user-generated events.
• @param value
• true if enabled false if not.
  */
  @LegacyMethod(name = "PB-ENABLED")
  public void setEnabled(logical value);

So I introduced these interfaces in order that widgets which have corresponding 4GL ActiveX components can use common key words.

This isn't correct either. The problem is that there are duplicated attributes with the same semantics. Now progress bar has the attributes PB-ENABLED and ENABLED (inherited from Widget). Besides, I'm not sure why the methods use the LegacyMethod annotation instead of LegacyAttribute.

Hynek Cihlar wrote:

Code review 4174a against latest trunk. > Please remove the interface IFontDisp. We already have an interface (and common implementation) to allow custom fonts, CustomFont.

Don't agree with this note too. CustomFont doesn't have required method because IDispFont uses decimal values for font size. Similar interfaces are not an issue but it helps to convert 4GL code properly. Simple adapters can be used for similar interfaces so actually only one of them can be implemented, for an example CustomFont.

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Code review 4174a against latest trunk. > Please remove the interface IFontDisp. We already have an interface (and common implementation) to allow custom fonts, CustomFont.

Don't agree with this note too. CustomFont doesn't have required method because IDispFont uses decimal values for font size.

CustomFont.setFontSize takes a NumberType so you can use the declaration already. If it makes sense to change the implementation BaseEntity.setFontSize to allow decimals, please do so.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Please remove the duplicate legacy attributes with the "OCX" prefix, like OCX-ENABLED and OCX-VISIBLE. Instead use the existing legacy interfaces that already declare these attributes. Also rename all the identifiers having the "ocx" substring. The ocx in the name may be misleading as the date picker widget has no connection to the OCX technology (other than that we model it after an existing OCX component).

Please remove the interfaces IActiveXUIComponent and IActiveXComponent. Again, we don't want to confuse the interface with ActiveX - a technology not used in the widget. OCX-ENABLED and OCX-VISIBLE should be replaced with ENABLED and VISIBLE from Widget interface. We already have getHWND() method in the interface CommonWidget. Move all the other methods in the DateTimePicker interface.
DateTimePickerConfig.DateTimePickerConfig() miss javadocs.

I don't agree with these notes at all. Please investigate the code of ProgressBar. It has these methods

Which part you don't agree with? The "OCX" confusing the interfaces or the duplicated attributes?

/**

• Return a value that determines whether a form or control can respond to user-generated events.
• @return true if enabled false if not.
  */
  @LegacyMethod(name = "PB-ENABLED")
  public logical isEnabled();

/**

• Set a value that determines whether a form or control can respond to user-generated events.
• @param value
• true if enabled false if not.
  */
  @LegacyMethod(name = "PB-ENABLED")
  public void setEnabled(logical value);

So I introduced these interfaces in order that widgets which have corresponding 4GL ActiveX components can use common key words.

This isn't correct either. The problem is that there are duplicated attributes with the same semantics. Now progress bar has the attributes PB-ENABLED and >ENABLED (inherited from Widget). Besides, I'm not sure why the methods use the LegacyMethod annotation instead of LegacyAttribute.

There is SENSITIVE attribute for 4GL widgets. ENABLED uses this rule

<rule>list.put(prog.kw_enabled , execLib("cr_descr", "Enabled"                    , "isEnabled"              , "setEnabled"             , true       ))</rule>

Yes, Enabled interface can be enough for conversion.

Does it make sense to implement VALUE attribute similarly for those widgets that use this attribute? DTP-VALUE and PB-VALUE look similar but use different types character, date/time and numeric respectively. PB-REFRESH and DTP-REFRESH key words represent a common method HRESULT Refresh(). These issues force me to use new common interfaces that look similar to the existed ones.

Sergey Ivanovskiy wrote:

There is SENSITIVE attribute for 4GL widgets. ENABLED uses this rule
[...]
and ProgressBar confuses me.

Yes, Enabled interface can be enough for conversion.

Does it make sense to implement VALUE attribute similarly for those widgets that use this attribute? DTP-VALUE and PB-VALUE look similar but use different types character, date/time and numeric respectively. PB-REFRESH and DTP-REFRESH key words represent a common method HRESULT Refresh(). These issues force me to use new common interfaces that look similar to the existed ones.

Yes, there will be generic attributes which are defined for all (or most) of the widgets, like ENABLED, VISIBLE, etc and specific ones for each widget.

ProgressBar and DatePicker widgets should support the generic attributes SCREEN-VALUE and INPUT-VALUE. But to allow conversion the OCX interface for getting and setting the value will be needed, too, I think. So assuming DTP-VALUE is part of the OCX interface, it will be needed on the widget (legacy) interface, too.

Now I am trying to extend UI of the date picker widget using a subclass of AbstractContainer with BorderLayout and a subclass of AbstractWidget, but the date picker widget location becomes broken, also there are issues to delegate a current focus from the container to its widget due to the tab item list logic. Also it needs to use Viewport and to implement ScrollableWidget interface for DateTimeContent so the date/time can be scrolled by LEFT/RIGHT keys if its viewport size is less than the content size. It would take more time than I expected for this UI part. I have no difficulties in common design issues but the particular usages take more working time. DateTimePickerGuiImpl has DateTimeView that has check box and/or drop down or up/down buttons and Viewport with DateTimeContent and separate dialog CalendarMonthPopup.

OK

I found that for a one button's click it was generated two ActionEvent. Is it desired behaviour?

Sergey Ivanovskiy wrote:

I found that for a one button's click it was generated two ActionEvent. Is it desired behaviour?

What are those events? It could be ENTRY and CHOOSE, which could be expected depending on your use case.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

I found that for a one button's click it was generated two ActionEvent. Is it desired behaviour?

What are those events? It could be ENTRY and CHOOSE, which could be expected depending on your use case.

The first ActionEvent is thrown by this code of ButtonGuiImpl

   protected void handleBasicEvents(KeyInput event)
   {
      // check if button is pressed by space and repaint it 
      if (isEnabled() && !keyPressed && event.id() == EventType.KEY_TYPED && 
          event.keyCode() == Key.VK_ENTER)
      {
         keyPressed = true; 
         repaint();
      } 

      // check if the button press is interrupted by this event which
      // is not another space type or a space release
      if (isEnabled() && keyPressed && 
          !(event.id() == EventType.KEY_TYPED && event.keyCode() == Key.VK_ENTER) &&
          !(event.id() == EventType.KEY_RELEASED && event.keyCode() == Key.VK_ENTER))
      {
         keyPressed = false;
         repaint();
      }

      if (isEnabled() && event.canProduceAction())
      {
         EventManager.postEvent(new ActionEvent(this, getCommand()));
      }
...............................................................

ButtonGuiImpl.handleBasicEvents(KeyInput) line: 905    
ButtonGuiImpl(AbstractButton<O,C>).processKeyEventCommon(KeyInput) line: 381    
ButtonGuiImpl(Button<O>).processKeyEvent(KeyInput) line: 524    
WindowGuiImpl(TitledWindow<O>).processKeyEvent(KeyInput) line: 306    
ButtonGuiImpl(AbstractWidget<O>).processEvent(Event) line: 1977    
ButtonGuiImpl(Button<O>).processEvent(Event) line: 488    
WindowGuiImpl(TitledWindow<O>).processEvent(Event) line: 275    
WindowGuiImpl.processEvent(Event) line: 1563    

   public void processEvent(Event event)
   {
      super.processEvent(event);

      if (event instanceof KeyInput)
      {
         KeyInput keyEvent = (KeyInput) event;
         if (isEnabled() && keyEvent.canProduceAction())
         {
            EventManager.postEvent(new ActionEvent(this, getCommand()));
            keyEvent.consume();
         }

         return;
      }
.........................................

ButtonGuiImpl(Button<O>).processEvent(Event) line: 495    
WindowGuiImpl(TitledWindow<O>).processEvent(Event) line: 275    
WindowGuiImpl.processEvent(Event) line: 1563    

It is a bug because two ActionEvent events are generated for one system event KeyInput: key=65535 action=-111 mode=regular source=[ButtonGuiImpl{-51, ""}]!

This code of AbstractWidget delegates to ancestor.processKeyEvent((KeyInput) evt) and this ancestor is the same WindowGuiImpl that processes this event by TitledWindow.processEvent

   public void processEvent(Event evt)
   {
      if (evt instanceof KeyInput)
      {
         Widget<O> ancestor = ancestor();

         if (ancestor != null)
            ancestor.processKeyEvent((KeyInput) evt);
      }
      else if (evt instanceof FocusEvent)
         processFocusEvent((FocusEvent) evt);
   }
............................

Sergey Ivanovskiy wrote:

It is a bug because two ActionEvent events are generated for one system event KeyInput: key=65535 action=-111 mode=regular source=[ButtonGuiImpl{-51, ""}]!

If two same ActionEvent@s are posted this doesn't seem to be correct. This is a guess but maybe @keyEvent.consume() should be invoked in handleBasicEvents?

It seems that it doesn't help because if the work flow goes down into ButtonGuiImpl.handleBasicEvents(KeyInput event), then two events will be generated according to the note 80. It seems that we need to overwrite handleEvent in ButtonGuiImpl. I am planning to use this work arround

   /**
    * It wraps around its super class implementation.
    * 
    * @param    event
    *           The processed event
    */
   @Override
   public void processEvent(Event event)
   {
      processedBySuperClass = true;
      super.processEvent(event);
      processedBySuperClass = false;
   }

Committed revision 11353 added calendar popup but it is not styled yet and some logic are not implemented. It uses simple GridLayout and LabelGuiImpl, but this class LabelGuiImpl is not fitted to the requirements. Planning to substitute it with another simple implementation that can align its text value within its bounds and to add calendar styles and code documentations and to fix the previous review issues.

The committed rev 11356 (4174a) fixed calendar popup functionality. Now its main functionality to select dates works properly. Working to implement these functionality

   /**
    * Returns the background color used to display the month portion of the dropdown calendar.
    * 
    * @return   The background color for the dropdown calendar given by the BGR encoding
    */
   integer getCalendarBackColor();

   /**
    * Sets the background color used to display the month portion of the dropdown calendar.
    * 
    * @param    calendarBackColor
    *           The background color for the dropdown calendar given by the BGR encoding
    */
   void setCalendarBackColor(integer calendarBackColor);

   /**
    * Returns the foreground color used to display text in the month portion of the dropdown calendar.
    * 
    * @return   The foreground color for the dropdown calendar given by the BGR encoding
    */
   integer getCalendarForeColor();

   /**
    * Sets the foreground color used to display text in the month portion of the dropdown calendar.
    * 
    * @param    calendarForeColor
    *           The foreground color for the dropdown calendar given by the BGR encoding
    */
   void setCalendarForeColor(integer calendarForeColor);

   /**
    * Returns the background color used to display the title portion of the dropdown calendar.
    * 
    * @return   The background color for the title portion of the dropdown calendar given by
    *           the BGR encoding.
    */
   integer getCalendarTitleBackColor();

   /**
    * Sets the background color used to display the title portion of the dropdown calendar.
    * 
    * @param    titleBackColor
    *           The background color for the title portion of the dropdown calendar given by
    *           the BGR encoding.
    */
   void setCalendarTitleBackColor(integer titleBackColor);

   /**
    * Returns the foreground color used to display the title portion of the dropdown calendar.
    * 
    * @return   The foreground color for the title portion of the dropdown calendar given by
    *           the BGR encoding.
    */
   integer getCalendarTitleForeColor();

   /**
    * Sets the foreground color used to display the title portion of the dropdown calendar.
    * 
    * @param    titleForeColor
    *           The foreground color for the title portion of the dropdown calendar given by
    *           the BGR encoding.
    */
   void setCalendarTitleForeColor(integer titleForeColor);

   /**
    * Returns the foreground color used to display the days at the beginning and end of
    * the dropdown calendar that are from previous and following months.
    * 
    * @return   The foreground color for the previous and following months of
    *           the dropdown calendar given by the BGR encoding.
    */
   integer getCalendarTrailingForeColor();

   /**
    * Sets the foreground color used to display the days at the beginning and end of the dropdown
    * calendar that are from previous and following months.
    * 
    * @param    trailingForeColor
    *           The foreground color for the previous and following months of
    *           the dropdown calendar given by the BGR encoding.
    */
   void setCalendarTrailingForeColor(integer trailingForeColor);

This is exciting! From the pictures, the text looks a bit "crowded". Can you make adjustments so that the text is not too close?

What is left to do in the task?

Do you mean to increase the column width for each day of week?

Yes, exactly right. It looks better, thanks.

BTW, in the #4174-1 the 4GL displays with even more space than you are using. I think it may make sense to increase it a bit more. I suspect they do this to support a wider range of languages.

I tested with these locale languages: French, Italian, German and Russian with FWD client, but didn't test 4GL with different languages. These pictures display FWD calendar popups:

It looks like it needs to fix minimal size of calendar so calendars for different languages look the same.

Sergey Ivanovskiy wrote:

It looks like it needs to fix minimal size of calendar so calendars for different languages look the same.

By any chance, does the width follow the maximum width of the combo's possible values (for a language)?

It looks like it needs to fix minimal size of calendar so calendars for different languages look the same.

Yes, probably although those don't look bad. Please check how it looks in the 4GL. If we have a big difference, then we should resolve it now so that it isn't considered a bug in the future. For example, this may happen if users complain or if automated testing breaks because it is too different.

Constantin Asofiei wrote:

Sergey Ivanovskiy wrote:

It looks like it needs to fix minimal size of calendar so calendars for different languages look the same.

By any chance, does the width follow the maximum width of the combo's possible values (for a language)?

Or maybe the width of the columns (from the day's name)?

The width of the date/time combo box is given by config.widthChars. If this value is not set, then it is calculated from the date/time pattern char width and the current font. It is done similarly to ComboBoxGuiImpl.

What is the status of this task? When are all items expected to be complete?

I need to use the common methods from ProggressBarWidget

   /**
    * Get mouse pointer.
    * 
    * @return mouse pointer
    */
   @LegacyMethod(name = "PB-MOUSE-POINTER")
   public integer getProgBarMousePointer();

   /**
    * Set mouse pointer.
    * 
    * @param   value
    *          new mouse pointer.
    */
   @LegacyMethod(name = "PB-MOUSE-POINTER")
   public void setProgBarMousePointer(integer value);

   /**
    * Set mouse pointer.
    * 
    * @param   value 
    *          new mouse pointer.
    */
   @LegacyMethod(name = "PB-MOUSE-POINTER")
   public void setProgBarMousePointer(int value);

   /**
    * Get mouse icon.
    * 
    * @return mouse icon
    */
   @LegacyMethod(name = "PB-MOUSE-ICON")
   public character getMouseIcon();

   /**
    * Sets a custom mouse icon.
    * 
    * @param   value
    *          user defined mouse icon
    */
   @LegacyMethod(name = "PB-MOUSE-ICON")
   public void setMouseIcon(character value);

   /**
    * Sets a custom mouse icon.
    * 
    * @param   value
    *          user defined mouse icon
    */
   @LegacyMethod(name = "PB-MOUSE-ICON")
   public void setMouseIcon(String value);   

because we shouldn't copy and paste the common code for these methods. I proposed to use IActiveXUIComponent and the base abstract class that implements these methods.

Yes, the code should be in a parent class. However, please rename any class that has ActiveX in the name. In FWD, there never will be ActiveX. Also, we should not be naming interfaces with a leading "I". That is a .NET thing which should not be brought over to Java.

Greg Shah wrote:

because we shouldn't copy and paste the common code for these methods. I proposed to use IActiveXUIComponent and the base abstract class that implements these methods.

Yes, the code should be in a parent class. However, please rename any class that has ActiveX in the name. In FWD, there never will be ActiveX. Also, we should not be naming interfaces with a leading "I". That is a .NET thing which should not be brought over to Java.

Sergey, also remove the "OCX" substring from Java and legacy method names of the added classes/interfaces. The OCX technology is no longer relevant in the converted sources.

https://proj.goldencode.com/projects/p2j/wiki/Progress_Indicator_Widget_and_OCX_Replacement control has OCX and OLE in legacy attributes:

         new Keyword("ocx-mouse-button"               ,  0, KW_OCX_MBTN, false),  // FWD extension, not a real 4GL keyword
         new Keyword("ocx-mouse-shift"                ,  0, KW_OCX_MSHT, false),  // FWD extension, not a real 4GL keyword
         new Keyword("ocx-mouse-x"                    ,  0, KW_OCX_MSX , false),  // FWD extension, not a real 4GL keyword
         new Keyword("ocx-mouse-y"                    ,  0, KW_OCX_MSY , false),  // FWD extension, not a real 4GL keyword

Sergey Ivanovskiy wrote:

https://proj.goldencode.com/projects/p2j/wiki/Progress_Indicator_Widget_and_OCX_Replacement control has OCX and OLE in legacy attributes:

[...]
Should these attributes be renamed?

Yes, these should be renamed and also: OCX-MOUSE-POINTER, OCX-MOUSE-ICON, OCX-REFRESH. The number of methods is low, so it shouldn't take long to rename them.

Please look at the committed revision 11367 (4174a) if the naming will be accepted, then I will proceed with another methods started with ole- prefix.

I'll let Hynek provide his feedback on the naming changes in rev 11367.

I will ask for this change: Please change DatePicker/DTPicker to Calendar. This is a better description of the control. I don't want to follow the Microsoft control name.

Greg Shah wrote:

I will ask for this change: Please change DatePicker/DTPicker to Calendar. This is a better description of the control. I don't want to follow the Microsoft control name.

Greg, I don't understand what should be done. In my changes there are no such entities. There are DateTimePicker, DateTimePickerWidget, DateTimePickerGuiImpl and DateTimePickerConfig. Also I was asked to change the code that I didn't add. I suspect that the proposed changes impact some test cases.

Please change "DateTimePicker" to "Calendar" everywhere (e.g. DateTimePickerWidget to CalendarWidget, DateTimePickerGuiImpl to CalendarGuiImpl, DateTimePickerConfig to CalendarConfig).

Similar changes would be made in progress.g. There should be no references to KW_DTPICKER or to DATETIME-PICKER...

Greg Shah wrote:

I'll let Hynek provide his feedback on the naming changes in rev 11367.

Please change REFRESH-EXT legacy method name to REFRESH-UI, this better matches the intention and the Java naming.

Please change MOUSE-POINTER-EXT to MOUSE-POINTER-NUM and MOUSE-ICON-EXT to MOUSE-ICON. These names are more descriptive. Also we have PB-MOUSE-ICON and PB-MOUSE-POINTER for PROGRESS-BAR widget. We should merge these names when the branches merge.

You removed the renamed methods/attributes from gaps rules. These should be still present.

Hynek Cihlar wrote:

Greg Shah wrote:

I'll let Hynek provide his feedback on the naming changes in rev 11367.

Please change REFRESH-EXT legacy method name to REFRESH-UI, this better matches the intention and the Java naming.

Please change MOUSE-POINTER-EXT to MOUSE-POINTER-NUM and MOUSE-ICON-EXT to MOUSE-ICON. These names are more descriptive. Also we have PB-MOUSE-ICON and PB-MOUSE-POINTER for PROGRESS-BAR widget.

OK

We should merge these names when the branches merge. You removed the renamed methods/attributes from gaps rules. These should be still present.

Please explain this idea more clearly. I removed them

@@ -2033,12 +2033,9 @@
          <rule>map.put(prog.kw_pb_appea, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_brsty, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_enabl, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
-         <rule>map.put(prog.kw_pb_msico, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
-         <rule>map.put(prog.kw_pb_mspnt, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_max,   rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_min,   rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_orien, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
-         <rule>map.put(prog.kw_pb_rfrsh, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_scrol, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_value, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_ole_drag, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>

@@ -2033,9 +2033,12 @@
          <rule>map.put(prog.kw_pb_appea, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_brsty, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_enabl, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
+         <rule>map.put(prog.kw_msicon,   rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
+         <rule>map.put(prog.kw_mspntnum, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_max,   rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_min,   rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_orien, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
+         <rule>map.put(prog.kw_rfrsh_ui, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_scrol, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_pb_value, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>
          <rule>map.put(prog.kw_ole_drag, rw.cvt_lvl_full    | rw.rt_lvl_full)</rule>

Greg Shah wrote:

Please change "DateTimePicker" to "Calendar" everywhere (e.g. DateTimePickerWidget to CalendarWidget, DateTimePickerGuiImpl to CalendarGuiImpl, DateTimePickerConfig to CalendarConfig).

Please review the committed changes in rev. 11368 (4174a).

Sergey Ivanovskiy wrote:

Please explain this idea more clearly. I removed them
[...]
because they were renamed to generic prog.kw_msicon, prog.kw_mspntnum and prog.kw_rfrsh_ui.

These generic attributes didn't have gap entries, so it is good you added them.

I committed these changes in revision 11368 (4174a).

The changes look good. Just please rename DTPicker (and any other old references to Calendar if any) in the comments as well.

Other than changing some comments, is there any remaining work in this task?

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Please explain this idea more clearly. I removed them
[...]
because they were renamed to generic prog.kw_msicon, prog.kw_mspntnum and prog.kw_rfrsh_ui.

These generic attributes didn't have gap entries, so it is good you added them.

I committed these changes in revision 11368 (4174a).

The changes look good. Just please rename DTPicker (and any other old references to Calendar if any) in the comments as well.

If I understand correctly, then the name DTPicker appears in the customer's code and is used to resolve this MS Date and Time Picker control in the 4GL code. Actually, this name can be changed, but in this case the customer's code must be changed and the corresponding ext-hints files. Does it make sense to change this name and what is the name to substitute it? (CALENDAR)?

Greg Shah wrote:

Other than changing some comments, is there any remaining work in this task?

I need to add history comments and move common methods with ole- legacy prefix into WidgetExtension and to ControlEntityExt, but it can complicate the future merge into the trunc.

Hynek, Greg, should I change this name OcxDragDrop to DragDropExt? Should this WidgetExtension interface extend drag and drop interface or should ControlEntityExt implements this drag and drop interface directly?

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

The changes look good. Just please rename DTPicker (and any other old references to Calendar if any) in the comments as well.

If I understand correctly, then the name DTPicker appears in the customer's code and is used to resolve this MS Date and Time Picker control in the 4GL code. Actually, this name can be changed, but in this case the customer's code must be changed and the corresponding ext-hints files. Does it make sense to change this name and what is the name to substitute it? (CALENDAR)?

No I meant FWD source codes, like "DTPicker" in ocx_conversion.rules and references to "* picker" in ui/**/Calendar* classes.

Sergey Ivanovskiy wrote:

Hynek, Greg, should I change this name OcxDragDrop to DragDropExt? Should this WidgetExtension interface extend drag and drop interface or should ControlEntityExt implements this drag and drop interface directly?

I would myself pick a different name for the Drag drop interface, as I don't like the generic tags like Ext. Possibly HasDragDrop or WithDragDrop. But this is a personal taste and if you like DragDropExt, then keep it.

Keeping the drag drop interface separate will give us more flexibility in future implementations of the interface. So I would implement this directly in ControlEntityExt.

I would like to notify that the huge number of files should be changed in order to remove OLE and OCX prefixes.

removed:

  src/com/goldencode/p2j/ui/OcxDragDrop.java
  src/com/goldencode/p2j/ui/OcxDragDropConfig.java
  src/com/goldencode/p2j/ui/OcxMouse.java
  src/com/goldencode/p2j/ui/OcxMouseConfig.java
  src/com/goldencode/p2j/ui/client/gui/OcxMouseHelper.java

  src/com/goldencode/p2j/ui/DragDropConfig.java
  src/com/goldencode/p2j/ui/MouseConfig.java
  src/com/goldencode/p2j/ui/WithDragDrop.java
  src/com/goldencode/p2j/ui/WithMouse.java
  src/com/goldencode/p2j/ui/client/gui/MouseHelper.java

Does it make sense to postpone these huge changes?

Sergey Ivanovskiy wrote:

Does it make sense to postpone these huge changes?

By huge you mean the five files? Or am I missing something?

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Does it make sense to postpone these huge changes?

By huge you mean the five files? Or am I missing something?

Yes, you are missing. There are more than these files. I don't understand these changes should be reviewed, shouldn't they?

Another question is how these files are related to this particular task that implements the calendar widget?

Sergey Ivanovskiy wrote:

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

Does it make sense to postpone these huge changes?

By huge you mean the five files? Or am I missing something?

Yes, you are missing. There are more than these files. I don't understand these changes should be reviewed, shouldn't they?

Which files are you referring to?

4174a
rules
convert
methods_attributes.rules (6 matches)
1,054: <rule>list.put(prog.kw_ole_drmd, execLib("cr_descr", "ProgressBar" , "getOleDropMode" , "setOleDropMode" , true ))</rule>
1,058: <rule>list.put(prog.kw_ole_x, execLib("cr_descr", "ProgressBar" , "getOleX" , "setOleX" , true ))</rule>
1,059: <rule>list.put(prog.kw_ole_y, execLib("cr_descr", "ProgressBar" , "getOleY" , "setOleY" , true ))</rule>
src
com
goldencode
p2j
comauto
ComOleDaemon.java (15 matches)
ComOleHelper.java (3 matches)
ComOleOps.java (2 matches)
NativeComObject.java (7 matches)
ui
chui
ThinClient.java (6 matches)
2,793: private ComOleDaemon cod = null;
3,126: cod = new ComOleDaemon(single);
24,206: OleDragDrop.clear();
24,222: return OleDragDrop.getData(format, id);
24,236: OleDragDrop.setData(value, format);
24,250: return OleDragDrop.getFormat(format);
client
gui
OleDragDrop.java (3 matches)
OleDragDropHelper.java (5 matches)
ProgressBarGuiImpl.java
ocx
DataObject.java
ControlEntityExt.java (12 matches)
LogicalTerminal.java
WithDragDrop.java (9 matches)
build.xml (2 matches)
487: <include name="com_goldencode_p2j_comauto_ComOleHelper.h"/>
1,231: <javah class="com.goldencode.p2j.comauto.ComOleHelper"

and these files
src/com/goldencode/p2j/ui/OcxDragDrop.java
src/com/goldencode/p2j/ui/OcxDragDropConfig.java
src/com/goldencode/p2j/ui/OcxMouse.java
src/com/goldencode/p2j/ui/OcxMouseConfig.java
src/com/goldencode/p2j/ui/client/gui/OcxMouseHelper.java

Were these files reviewed before they were committed in the trunc?

Also there is a package com.goldencode.p2j.ui.ocx. Please explain the purpose of these changes/renamings in this task?

Sergey Ivanovskiy wrote:

Also there is a package com.goldencode.p2j.ui.ocx. Please explain the purpose of these changes/renamings in this task?

I didn't mean to rename the OLE prefixes. These in the context of the OLE drag & drop are valid. I only meant to rename the OCX and ActiveX from your changes where these names don't make sense anymore because OCX and ActiveX are no longer relevant.

Sergey Ivanovskiy wrote:

OK. The previous notes confuse me.

Some OCX/COM references are desired, like those in the package com.goldencode.p2j.ui.ocx. This contains classes that implement COM interfaces that we need to expose by COM objects implemented (or emulated) in FWD and by widgets that do expose part of their behavior with COM interfaces (like OLE drag & drop). These are fine and desired to refer to the OCX technology.

For CALENDAR widget this is different. We want the widget to be a legacy widget. The fact that we are capable to convert a MS date time picker OCX to CALENDAR widget is irrelevant for anybody who will use CALENDAR from scratch. In other words the conversion of MS date time picker OCX to CALENDAR and the CALENDAR widget implementation are functionally two distinct pieces.

Please review these changes, the committed revision 11372 (4174a) that added history entries.

Code review 4174a revisions 11369..11372.

Missing file history entries:
rules/convert/ui_statements.rules
rules/gaps/expressions.rules
src/com/goldencode/p2j/ui/OcxDragDrop.java
src/com/goldencode/p2j/ui/ProgressBarConfig.java

Wrong history entry number in src/com/goldencode/p2j/uast/progress.g.

Please review the fixed code the committed revision 11373 (4174a).

Sergey Ivanovskiy wrote:

Please review the fixed code the committed revision 11373 (4174a).

The changes are good. I have no more open concerns for the branch.

What testing (conversion and runtime) is needed before this can be merged to trunk?

Greg Shah wrote:

What testing (conversion and runtime) is needed before this can be merged to trunk?

The customer's code must be reconverted and checked because this branch is from the trunc where there is a known regression related to usages of ActiveX components in 4GL code, where the corresponding MS Date and Time control are used. The other tests are from testcases. Planning to do manual testing from testcases project today.

The committed revision 11374 (4174a) fixed progressbar widget issues and this test from testcases must be fixed due to 4174a changes. Also all these tests require pbar.wrx file to be present in the client application path. It can be any file and finally I used this file test-date-time-picker-1.wrx renamed to pbar.wrx

=== modified file 'uast/progressbar/progress_bar_test6.p'
--- uast/progressbar/progress_bar_test6.p    2019-10-29 14:11:16 +0000
+++ uast/progressbar/progress_bar_test6.p    2020-02-26 13:07:04 +0000
@@ -48,10 +48,10 @@
 progBar:pb-Max = 100.
 progBar:pb-Min = 0.
 progBar:pb-Value = 30.
-progBar:pb-Mouse-Pointer = 3.
+progBar:Mouse-Pointer-Num = 3.
 /* progBar:pb-Mouse-Icon = "user-cursor.png". */
 progBar:ole-Drag().
-progBar:pb-Refresh().
+progBar:Refresh-ui().

 /* progress-bar mouse events */
 on "mouse-click" of progBar do:
@@ -207,7 +207,7 @@

 do iCount = 1 to progBar:pb-Max :
     progBar:pb-Value = iCount.
-    progBar:pb-Refresh( ).
+    progBar:Refresh-ui( ).
 end.
 end procedure.

I don't understand why the .wrx file is needed. If you configure the .ext-hints file, the LoadControls will be converted properly without any .wrx.

But this test uast/progressbar/progress_bar_test6.p doesn't have control_load procedure and hence it can be used without pbar.wrx

Yes, I missed this option in the hint file progress_bar_test.p.ext-hints

<drop-procedure name="control_load" />

During the test_bar_test2.p I found the conversion issues and fixed them so the ProgressBar can be wrapped into Enabled and into WidgetExtension in order to access to target methods.

Please review these changes: the committed revision 11376 (4174a).

Committed revision 11377 (4174a) fixed the conversion for new calendar widget when common methods of OcxMouse and OcxDragDrop interfaces are used.
Although the Hotel GUI and the large customer GUI application were converted successfully and were tested manually, I am still fixing uast/calendar/test-calendar.p tests.

There are issues related to SCREEN-VALUE and to setDateTimeValue due to date/time format issues. 4174a will be ready for the trunc as soon as these two issues will be fixed. Planning to fix them tomorrow and commit to the trunc if there are no objections.

No objections.

Committed revision 11378 (4174a) fixed SCREEN-VALUE for new calendar widget.

Committed rev 11379 (4174a) fixed DateTimeValue widget value to accept ISO date and time string value. Planning to merge this version to the trunc. Please review.

Code review 4174a revisions 11373..11379.

<action>map6.put("UpDown" , "UpDown")</action> is redundant in ocx_conversion.rules

Otherwise the changes seem OK.

Hynek Cihlar wrote:

Code review 4174a revisions 11373..11379.

<action>map6.put("UpDown" , "UpDown")</action> is redundant in ocx_conversion.rules

Otherwise the changes seem OK.

Committed rev 11379 branch 4174a into the trunc as the revision 11343. I will remove this rule from the map with the first branch that was planned into the trunc.

This task branch 4174a has been merged into the trunc as rev 11343 and archived.

These MS Date/Time Picker features are not implemented yet:

1) to support MS CustomFormat https://docs.microsoft.com/en-us/windows/win32/controls/date-and-time-picker-controls#custom-formats
with its callback fields. Now Calendar:CustomFormat should support java date/time format strings.

2) Callback fields

In addition to the standard Format Strings and body text, you can also define certain parts of the display as Callback fields. These fields can be used to query the user for information. To declare a callback field, include one or more "X" characters (ASCII Code 88) anywhere in the format string. You can create callback fields that have a unique identity by repeating the "X" character. Thus, the format string "XX dddd MMM dd', 'yyy XXX" contains two unique callback fields, "XX" and "XXX". Like other DTP control fields, callback fields are displayed in left-to-right order based on their location in the format string.

When the DTP control parses the format string and encounters a callback field, it sends DTN_FORMAT and DTN_FORMATQUERY notification codes. The format string element corresponding to the callback field is included with the notifications to allow the receiving application to determine which callback field is being queried. The owner of the control must respond to these notifications to ensure that the custom information is properly displayed.

3) The calendar date/time fields can be edited by typing valid numbers. Now it is not implemented and the field value can be changed only by pressing UP/DOWN keys or by selecting the calendar day on the calendar popup or by clicking on the Up/Down buttons.

Committed revision 11420 (4207a) fixed the custom format and color setters, removed extra elements from Calendar widget methods map. These changes were tested with testcases project.

Sergey: Please write up the documentation for this widget in CALENDAR Widget and OCX Replacement. You can use the same format used by ProgressBar or TreeView.

I see that there is a good start on the documentation. Thank you!

I sent the link to some customers for review. They have provided the following feedback:

Looks nice, but the documentation provokes some questions. Because it nicely documents FormatStyle but does not mention other properties or events.

The example sourcecode shows that the widget has a DateTimeValue property. I wonder what datatype it is: I see an example where it is assigned a datetime value and I also see an example where it is assigned a character value (in iso format for datetime). The statement:

MESSAGE dtWidget3:DateTimeValue VIEW-AS ALERT-BOX.

does not give me a hint to which datatype the value can be assigned, because MESSAGE can display all datatypes. I hope it is not a character string, because converting a string to a date is not trivial, can easily go wrong if the app only needs the date part but the string represents a datetime with a time near midnight in a different timezone...

I see two examples where the calender widget is used as a time picker without a date, so only a time is displayed (11:23 PM and 12:00 AM). So I guess it only has a time value, is there a TimeValue property? Or a DateTimeValue in which the date is meaningless? What is screen-value in that case?

Is screen-value a character? Is it dependent on locale (if so do we know what the local format is at the client?), or the selected format if FormatStyle=3, or is it always the iso string?

Does the widget fire an event when the user has changed the value? What happens when the user entered an invalid value?

Is there a property to make the week start on Monday instead Sunday? Or as we say: maandag instead zondag. Does it have translations?

What is the purpose of the "checked" property?

Please update the documention to answer these questions. Make sure all calendar-specific attributes and methods are documented.

Working to improve the documentation of calendar. The current version of Calendar can convert DateTimeValue value from datetime type or character type. For the character type the format should be according to the current 4GL datetime format or to be ISO date time string but the returned value is always a character string given in the current format. For these formats: long, short and time the locale settings are used, but the custom format the date/time components are only localized automatically by java date time api. Here I followed the original MS date time picker activex control. Now the changed value event is not fired and incorrect value can't be inserted because the control keys are mouse buttons and LEFT/RIGHT and UP/DOWN arrows. The calendar's beginning day of week is not customized. These 3 requirements can be easily implemented if it is requested.

I think the VALUE-CHANGED event and the custom beginning day of week will be needed.

but the custom format the date/time components are only localized automatically by java date time api

I don't understand what you mean by this. Are you saying that the localization will be based on the JVM default locale?

The custom format is implemented exactly according to this documentations https://docs.oracle.com/javase/tutorial/i18n/format/simpleDateFormat.html.
Let me use this example. If CustomFormat="What month is now? MMMM", then the current date time will be displayed in this way. "What month is now?" is not translated but instead of "MMMM" the month name will be substituted using the current user's locale settings. If Italian language is used, then the current month will be "marzo".

Vladimir Tsichevski wrote:

Is it possible to run this the calendar or its parts as a pure-client widget?

I used the uast/calendar/test-calendar-1.p source as an example. This original example compiles and run Ok, so I've tried to extract the "pure client" part and run the widget in a dialog on the client. This is the code I used:
[...]

The code compiles and runs Ok, but the result is as follows:

And. if I click with mouse in the visible rectangle, I got NPE inside the mousePressed() method at the following line (displayedDateTime is null at this point):

[...]

Could you send me the reference to your test case or this failed test case. The code should work I will test it tomorrow and write what are the root causes.

Vladimir, it can be that you should invoke CalendarGuiImpl.this.initialize(WidgetId id, CalendarConfig cfg) artificially but I can't give helpful answer without the code.
For an example ButtonGuiImpl, it needs to invoke initialize, when ButtonGuiImpl is used only on the client side.

      WidgetId wid = WidgetId.nextID();

      ButtonGuiImpl button = widgetsFactory.createButton(wid, null, name + wid.asInt());

      button.config().widthPixels  = width;
      button.config().heightPixels = height;
      button.config().widthChars  = cc.widthFromNative(width);
      button.config().heightChars = cc.heightFromNative(height);
      button.config().enabled = true;
      button.config().visible = true;
      button.config().fixedWidth = true;
      button.config().fixedHeight = true;
      button.config().x = 0;
      button.config().y = 0;

      imageSetter.accept(button);

      button.initialize(wid, null);

Vladimir Tsichevski wrote:

Sergey Ivanovskiy wrote:

Vladimir, it can be that you should invoke CalendarGuiImpl.this.initialize(WidgetId id, CalendarConfig cfg) artificially

Yes, now I can see the text part of the widget with no button, and no exception is raised anymore:

And no more exception is raised when I click the area with the mouse (nothing happens).

but I can't give helpful answer without the code.

You will find the code in customers repository, around the grid package. The code to run the dialog are as follows:
[...]

I found the code for AbstractDialog.java, but couldn't find TestCalendarWidget. Could you post it for me? It seems that the code should work and there are some issues related to buttons. For the formatStyle = 1 the popup button with arrow down should be created. Probably the root cause issues are not in CalendarGuiImpl itself.

I guessed that visibility of buttons (popup, up and down buttons) are set by afterConfigUpdate() thus the popup button is hidden. Vladimir, please post your test case and I will change the code to add new methods to CalendarGuiImpl that set up buttons visibility.

It looks like this code can be tried

WidgetId calendarId = WidgetId.nextID();
      CalendarConfig cfgBeforeUpdate = new CalendarConfig(calendarId);
      CalendarConfig cfg = new CalendarConfig(calendarId);
      cfg.widthChars = 20;
      cfg.heightChars = 20;

      cfg.row = 3.62;
      cfg.column = 5;
      cfg.heightChars = 1.2;
      cfg.widthChars = 30;
      CalendarGuiImpl calendarWidget = new CalendarGuiImpl(calendarId, cfg);
      /* calendar methods and attributes */
      cfg.formatStyle = 1;

      calendarWidget.initialize(calendarId, cfg);

      calendarWidget.afterConfigUpdate(cfgBeforeUpdate);

      calendarWidget.setValue(new character(datetimetz.now()));

I didn't note that the widget value should be set via cfg.dateTimeValue because calendarWidget.setValue was not implemented.

Vladimir, what is the purpose of such tests? You used org.eclipse.jdt.annotation.Nullable annotation. Are these tests to detect possible NPE?

I tried to run AbstractDialog.run(new TestCalendarWidget(), "Test", 200, 200); and it failed

Exception in thread "main" java.lang.NullPointerException

    at com.goldencode.p2j.ui.ConfigManager$1.initialValue(ConfigManager.java:137)
    at com.goldencode.p2j.ui.ConfigManager$1.initialValue(ConfigManager.java:1)
    at com.goldencode.p2j.security.ContextLocal$Fallback.initialValue(ContextLocal.java:869)
    at java.lang.ThreadLocal.setInitialValue(ThreadLocal.java:180)
    at java.lang.ThreadLocal.get(ThreadLocal.java:170)
    at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:459)
    at com.goldencode.p2j.security.ContextLocal.get(ContextLocal.java:430)
    at com.goldencode.p2j.ui.ConfigManager.getInstance(ConfigManager.java:242)
    at com.goldencode.p2j.ui.client.widget.AbstractWidget.<init>(AbstractWidget.java:339)
    at com.goldencode.p2j.ui.client.widget.AbstractContainer.<init>(AbstractContainer.java:249)
    at com.goldencode.p2j.ui.client.Frame.<init>(Frame.java:1041)
    at com.goldencode.p2j.ui.client.gui.FrameGuiImpl.<init>(FrameGuiImpl.java:272)
    at test.AbstractDialog.<init>(AbstractDialog.java:42)
    at test.TestCalendarWidget.<init>(TestCalendarWidget.java:20)
    at test.TestCalendarWidget.main(TestCalendarWidget.java:56)

It seems that CalendarGuiImpl should be refactored in order to be run as a client. afterConfigUpdate is important in this version of CalendarGuiImpl. Please help to understand what are requirements in order to run these tests without exceptions?

The committed rev 11547 fixed unset background and foreground colors used for selected text.This issue was found when I checked ./calendar/test-calendar-1.p (testcases project) with Classic theme.

Sergey Ivanovskiy wrote:

I found the code for AbstractDialog.java, but couldn't find TestCalendarWidget.

I forgot to add the file to the repo, sorry.
I did it now.

Sergey Ivanovskiy wrote:

It looks like this code can be tried
[...]

I changed the code, now I get

java.lang.IllegalStateException: Widget null -53 is not attached to a Window instance.

The top of stacktrace:

Thread [main] (Suspended (exception IllegalStateException))
    CalendarGuiImpl(AbstractWidget<O>).window() line: 463
    CalendarGuiImpl.updateSize() line: 1073
    CalendarGuiImpl.afterConfigUpdate(CalendarConfig) line: 407
    TestCalendarWidget.initialize(WidgetId, FrameConfig) line: 36
    AbstractDialog.run(AbstractDialog, String, int, int) line: 30
...

Sergey Ivanovskiy wrote:

Vladimir, what is the purpose of such tests? You used org.eclipse.jdt.annotation.Nullable annotation. Are these tests to detect possible NPE?

It is part of statical Java code analysis. Let you much better understanding of your code (be absolutely sure which types are nullable, and which are not), and (almost) forget about NPEs.

I use annotations in org.eclipse.jdt.annotation, which is default in Eclipse, but any suitable annotations could be used for that purpose. Very sad, no similar technique is used in FWD.

Sergey Ivanovskiy wrote:

Exception in thread "main" java.lang.NullPointerException
[...]

I have not managed to reproduce this very problem.

Sergey Ivanovskiy wrote:

The committed rev 11547 fixed unset background and foreground colors used for selected text.This issue was found when I checked ./calendar/test-calendar-1.p (testcases project) with Classic theme.

I've upgraded to this version. Still getting the IllegalStateException.

Sergey Ivanovskiy wrote:

It seems that CalendarGuiImpl should be refactored in order to be run as a client.

Yes, it shall be able to run without any server interaction-related calls like afterConfigUpdate.

Besides, I would like to use the CalendarPopup as a separate widget in my own pop-up window. The nature of this widget does not restrict its usage as pop-up only.

Vladimir, from CalendarGuiImpl.updateSize():

      // The current widget window
      Window<?> window = window();

      if (window == null)
      {
         // the widget is not attached, just return
         return;
      }

It is a requirement for this widget to be attached to a FWD client-side window; otherwise, window() will fail. Why is this needed: in 4GL, the font-table (used to compute widget implicit dimensions, draw text, etc) lives at the window.

Are you trying to test the CalendarGuiImpl widget in a standalone Java program, de-coupled completely from the main FWD logic? If so, I don't think that will be possible, neither for this or other widgets.

Vladimir Tsichevski wrote:

Sergey Ivanovskiy wrote:

The committed rev 11547 fixed unset background and foreground colors used for selected text.This issue was found when I checked ./calendar/test-calendar-1.p (testcases project) with Classic theme.

I've upgraded to this version. Still getting the IllegalStateException.

Yes, IllegalStateException is thrown by window(). It is not correct to invoke calendarWidget.afterConfigUpdate(cfgBeforeUpdate); at this step when its parent is null so it was incorrect way. Please help with instructions how to run these similar tests from Eclipse. I guess that it requires to setup a client session. I just tried to use use AbstractDialog and create a subclass TestCalendarWidget. For tests I used client and server, but for the client it needs to create some session object. What are your requirements and task that requires you to run CalendarGuiImpl. What is your test environment?

Constantin Asofiei wrote:

Vladimir, from CalendarGuiImpl.updateSize():
[...]

Are you trying to test the CalendarGuiImpl widget in a standalone Java program, de-coupled completely from the main FWD logic?

No, I run FWD server and client test programs, and I create a simpl test dialog with the widget in question.
It is just not the right state of the dialog to call this code.

Do you have the default window shown? Do a message "start." to display the default window, at the beginning of the program. Maybe is that the default window was not created yet.

Vladimir, where can I see the source code for the AbstractDialog and TestCalendarWidget files?

Thread [main] (Suspended (exception IllegalStateException))
    CalendarGuiImpl(AbstractWidget<O>).window() line: 463
    CalendarGuiImpl.updateSize() line: 1073
    CalendarGuiImpl.afterConfigUpdate(CalendarConfig) line: 407
    TestCalendarWidget.initialize(WidgetId, FrameConfig) line: 36
    AbstractDialog.run(AbstractDialog, String, int, int) line: 30

Vladimir Tsichevski wrote:

Sergey Ivanovskiy wrote:

It seems that CalendarGuiImpl should be refactored in order to be run as a client.

Yes, it shall be able to run without any server interaction-related calls like afterConfigUpdate.

Besides, I would like to use the CalendarPopup as a separate widget in my own pop-up window. The nature of this widget does not restrict its usage as pop-up only.

Yes, agree. it can be used. Now I understand the issue. Please test your changes with these tests from uast/calendar from testcases project to be sure that the code refactoring is correct:

./calendar/test-date-time-picker-1.w
./calendar/test-calendar-1.p
./calendar/test-calendar-2.p

Vladimir, extracting CalendarPopup can be done if we take into account its parent and its CalendarMonthModel, font resolver, color resolver and look and feel constants.

      public CalendarPopup()
      {
         super(new StackLayout<GuiOutputManager>(Direction.VERTICAL), new LineBorderGuiImpl());
         setId(WidgetId.nextID());
         setParent(CalendarGuiImpl.this);

         model = new CalendarMonthModel(displayedDateTime.toLocalDate());

Vladimir, please try to set the calendar's parent via calendarWidget.setParent(this), in TestCalendarWidget.

Constantin Asofiei wrote:

Vladimir, please try to set the calendar's parent via calendarWidget.setParent(this), in TestCalendarWidget.

Now I've got an empty window :(

I will do the following:

1. Copy the CalendarGuiImpl.java to the dxgrid source tree, may be rename it temporary to avoid confusion;
2. Discover how it works;
3. Refactor the code toward meeting my requirements;
4. Try to put the code back to FWD source tree.

Vladimir Tsichevski wrote:

Constantin Asofiei wrote:

Vladimir, please try to set the calendar's parent via calendarWidget.setParent(this), in TestCalendarWidget.

Now I've got an empty window :(

Did you set visible the calendar widget?

Vladimir Tsichevski wrote:

Constantin Asofiei wrote:

Vladimir, please try to set the calendar's parent via calendarWidget.setParent(this), in TestCalendarWidget.

Now I've got an empty window :(

I will do the following:

1. Copy the CalendarGuiImpl.java to the dxgrid source tree, may be rename it temporary to avoid confusion;

I didn't work with this environment. Do you have instructions how to setup it?

1. Discover how it works;
2. Refactor the code toward meeting my requirements;

What are your requirements? I can do some work for you if you have no objections.

Constantin Asofiei wrote:

Vladimir Tsichevski wrote:

Constantin Asofiei wrote:

Vladimir, please try to set the calendar's parent via calendarWidget.setParent(this), in TestCalendarWidget.

Now I've got an empty window :(

Did you set visible the calendar widget?

No, I thought any widget is visible by default. Will try it.

The argument of the following constructor is not used:

      public CalendarMonthModel(LocalDate selected)

Meanwhile, there exists a field selected which is left uninitialised.

I suspect this is a bug.

Vladimir Tsichevski wrote:

The argument of the following constructor is not used:
[...]

Meanwhile, there exists a field selected which is left uninitialised.

I suspect this is a bug.

Now I noticed the set() call in the same constructor, so the problem does not exist.

The following code looks suspicious (line 1168):

      if (Utils.equals(oldValue, config.dateTimeValue))
      {
         fireValueChanged();
      }

If the new value is the same as the previous one, why fire an event?

Vladimir Tsichevski wrote:

The following code looks suspicious (line 1168):

[...]

If the new value is the same as the previous one, why fire an event?

Yes, it is incorrect code, it seems that the correct code should be !Utils.equals(oldValue, config.dateTimeValue).

Planning to refactor CalendarGuiImpl in order that CalendarPopup can be used independently and to commit today.

Committed revision 11553 (4231b) Fixed #4174-191, extracted CalendarPopup, CalendarMonthModel, added ButtonFactory. Please review the refactored code.

Sergey Ivanovskiy wrote:

Committed revision 11553 (4231b) Fixed #4174-191, extracted CalendarPopup, CalendarMonthModel, added ButtonFactory. Please review the refactored code.

Why did you implement the CalendarMonthModel as a public class, it is used exclusively in the CalendarPopup (so it does not need to be public), and as a single instance. I would inline all the CalendarMonthModel fields and methods into CalendarPopup, it would eliminate the need for multiple dereferences, and make things much simpler.

Vladimir Tsichevski wrote:

Sergey Ivanovskiy wrote:

Committed revision 11553 (4231b) Fixed #4174-191, extracted CalendarPopup, CalendarMonthModel, added ButtonFactory. Please review the refactored code.

Why did you implement the CalendarMonthModel as a public class, it is used exclusively in the CalendarPopup (so it does not need to be public), and as a single instance. I would inline all the CalendarMonthModel fields and methods into CalendarPopup, it would eliminate the need for multiple dereferences, and make things much simpler.

Yes, agree. I didn't observe it at the beginning, thanks, I will move it to CalendarPopup.

Fixed in rev 11555 (4231b).

I suppose declarations like the following should read leading instead of trailing since we mean something before, not after?

   /**
    * The end day of the trailing days before the current calendar month
    */
   private LocalDate endFirstTrailingDay;

Also all panels in CalendarPopup except the daysInMonthPanel should not be widget fields, since they are referenced in the CalendarPopup constructor only.

Task branch 4231b has been merged to trunk as revision 11347.

Sergey, how can I run test-date-time-picker-1.w in 4GL?

Constantin Asofiei wrote:

Sergey, how can I run test-date-time-picker-1.w in 4GL?

Constantin, please read this thread from #4174-8. I checked this test so it should work with 32-bit version and mscomct2.ocx (#4174-40).

C:\Progress\OE116_32\bin\prowin32.exe -cpstream UTF-8 -cpinternal UTF-8 -basekey "INI" -ininame progress11.ini -debug -p _edit.p -debug

• use an 'invisible' calendar and the dmy date format via SESSION:DATE-FORMAT = "dmy". I mean, I don't care what the calendar UI looks like, I want to know how the VALUE attribute behaves in 4GL.
• set the VALUE attribute to TODAY
• read back the VALUE attribute and write it:
  • as a string
  • convert it to def var d as date. d = DATE(...:VALUE) and write STRING(d).

Please let me know what is shown for the last two points.

Sergey, the shown VALUE is not ISO-8601 format; why was this assumption made, that the string read from the VALUE attribute is ISO-8601?

Also, you mention date time picker should have style format - how is this set? Does this affect both the write and read of the VALUE attribute?

I think we need some more tests, which checks the VALUE attribute read/write, with different styles and SESSION:DATE-FORMAT.

Please write a test which can be automated, with no interactive UI (or no UI at all, if possible) and just has some logic like:

// set style, format, etc
// assign VALUE attribute
if h:value <> "expected value" then message "failed case ..." h:value.

Constantin Asofiei wrote:

Sergey, the shown VALUE is not ISO-8601 format; why was this assumption made, that the string read from the VALUE attribute is ISO-8601?

Also, you mention date time picker should have style format - how is this set? Does this affect both the write and read of the VALUE attribute?

Please look at #4174-64 and #4174-53. The idea was to hold the general date time value in dateTimeValue and to display and return the format according to its FORMAT. If FORMAT = 3, then it uses CustomFormat string attribute to represent its dateTimeValue.

It seems that SESSION:DATE-FORMAT is applied only (has impacts only) to DATE, DATETIME and DATETIME-TZ and it is not related to IDTPicker.

In my use case in #4947, there is no usage of the FORMAT attribute for the calendar. So it uses the default calendar format.

Even so, I don't understand what made you decide that the VALUE attribute should be in ISO-8601 format:

   /**
    * Sets the current date and time value.
    * 
    * @param    value
    *           The new current date and time value
    */
   @Override
   public void setDateTimeValue(datetime value)
   {
      config.dateTimeValue = value.getIsoDate();

      setScreenValue(value);

      pushWidgetAttr(new String[] {"dateTimeValue", "formatStyle"},
                     new Object[] {config.dateTimeValue, config.formatStyle});
   }

I would like to propose to extend DATE, DATETIME and DATETIME-TZ in FWD in order to except ISO8601 date and time strings.

Sergey Ivanovskiy wrote:

I would like to propose to extend DATE, DATETIME and DATETIME-TZ in FWD in order to except ISO8601 date and time strings.

Not until I understand how calendar's VALUE attribute is actually working in 4GL. Can you provide step-by-step instructions to make your testcase with the calendar work in 4GL?

Constantin, OK, I will try to write these instructions. It seems that finally it was due to the fact that VALUE is mapped to DateTimeValue indirectly via conversion scripts. Thus it becomes an issue.

I think that the date time picker formats its value according to the locale settings. So I can change locale and its default format will be changed and the date and time will be displayed according to this format. I followed this design. There is no default FORMAT. It is provided via application builder or set explicitly via FORMAT.

/**
    * Displays the date in long format. The default format string for this style is defined by
    * the current locale settings, which produces output like "Saturday, February 1, 2020". 
    */
   public int DTS_LONGDATEFORMAT  = 0;

   /**
    * Displays the date in short format. The default format string for this style is defined by
    * the current locale settings, which produces output like "02/01/20". 
    */
   public int DTS_SHORTDATEFORMAT = 1;

   /**
    * Displays the time. The default format string for this style is defined by the current locale
    * settings, which produces output like "10:29:00 AM". 
    */
   public int DTS_TIMEFORMAT      = 2;

   /**
    * Displays date and time using the format string specified in the 'CustomFormat' property.
    */
   public int DTS_CUSTOMFORMAT    = 3;

Looks like the VALUE attribute for the calendar is using OS DATE format - I don't know about the time portion of the VALUE string, as I can't make it report anything other than 00:00:00.000. Sergey, is there a way to change the TIME in the calendar?

• conversion of VALUE emits as a CalendarWidget.getValue/setValue instead of the get/setDateTimeValue.
• the VALUE setter delegates to setDateTimeValue.
• the VALUE getter uses the dateTimeValue and formats it correctly, as in <os-date-format><space-character>00:00:00.000.
• to emulate the OS date format, we can either configure it in the directory.xml or set the LANG system property before starting the server (but I don't know if this will work - and it may have other side-effects).

to emulate the OS date format, we can either configure it in the directory.xml or set the LANG system property before starting the server (but I don't know if this will work - and it may have other side-effects).

This definitely will have side effects. We don't want to override things just for this case. It is not clear to me what is being lost here.

Greg Shah wrote:

It is not clear to me what is being lost here.

The OS date format affects when the VALUE attribute is set or read. But I think the SESSION:DATE-FORMAT affects it, too, because if this is not in sync with the OS format, then I get some weird results... I need to build a matrix of the 4GL and OS date formats, and write various tests on it, and see what happens.

Constantin Asofiei wrote:

Greg Shah wrote:

It is not clear to me what is being lost here.

The OS date format affects when the VALUE attribute is set or read. But I think the SESSION:DATE-FORMAT affects it, too, because if this is not in sync with the OS format, then I get some weird results... I need to build a matrix of the 4GL and OS date formats, and write various tests on it, and see what happens.

Constantin, this ocx control had been created without any knowledge about 4GL, it should implement a user's date and time locale settings. It was described in the docs. It is more clear to implement new constructors for DATE and DATETIME. #4174-212

Sergey Ivanovskiy wrote:

Constantin, this ocx control had been created without any knowledge about 4GL, it should implement a user's date and time locale settings.

This FWD CALENDAR widget is a replacement for the OS-native OCX calendar control. And must behave the same. It's not some new widget which/control exists only in FWD.

It was described in the docs. It is more clear to implement new constructors for DATE and DATETIME. #4174-212

We can't add ISO-8601 support to the legacy 4GL DATE! or DATETIME function as this is not supported by 4GL. For example:

def var d as date.
d = date("2020-11-05T15:36:16+00:00").

More, the VALUE attribute must behave in FWD as it behaves in the OCX - its read/write must be the same. For example, if FWD uses ISO-8601 format for it, and some business logic expects to do some processing like 'split on first space', will not work. Or the DATE(dt-calendar:VALUE), which is not working in FWD.

Constantin Asofiei wrote:

Sergey Ivanovskiy wrote:

Constantin, this ocx control had been created without any knowledge about 4GL, it should implement a user's date and time locale settings.

This FWD CALENDAR widget is a replacement for the OS-native OCX calendar control. And must behave the same. It's not some new widget which/control exists only in FWD.

It was described in the docs. It is more clear to implement new constructors for DATE and DATETIME. #4174-212

We can't add ISO-8601 support to the legacy 4GL DATE! or DATETIME function as this is not supported by 4GL. For example:
[...]
does not work in 4GL (an ERROR condition is raised).

More, the VALUE attribute must behave in FWD as it behaves in the OCX - its read/write must be the same. For example, if FWD uses ISO-8601 format for it, and some business logic expects to do some processing like 'split on first space', will not work. Or the DATE(dt-calendar:VALUE), which is not working in FWD.

For the client side of CalendarWidget getValue() returns localized version of the current FORMAT. I suppose that dateTimeValue is updated by the client side, so it seems the most direct way is to call to the client side.

The uast/calendar/calendar-formats.txt.4gl and uast/calendar/calendar-value-tests.p shows that the VALUE attribute for the legacy dtpicker OCX control fallows the 'datetime' 4GL EXPORT formatting.

The fix is in 3821c rev 11798 and requires reconversion. Sergey, please do test some of your calendar 4GL tests.

One more thing to fix: the value displayed is formatted using the LANG for the machine where the FWD client process was started, even if in directory.xml the date is configured as dmy.

I'm changing this to use the directory date format and do not rely on the JVM's date format.

1. mdy, compatible with en_US, Locale.US in Java
2. dmy, compatible with en_UK, Locale.GB and others in Java
3. ymd, compatible with en_CA, Locale.CANADA and others in Java

1. configure the locale (like en_US, en_GB, fr_FR), in the directory and use that in CalendarGuiImpl.locale.
2. start the FWD server with the proper LANG - this will solve the Web clients, as the LANG is inherited. For the Swing client, this needs to be configured explicitly in the script.

From these two, I'd go with the first version, configure the locale in the directory.

configure the locale (like en_US, en_GB, fr_FR), in the directory and use that in CalendarGuiImpl.locale

I'm OK with this. Please name the entry something that is clearly associated with the calendar control.

Greg, is it acceptable for the 'today date' to appear like this?

Yes.

Greg Shah wrote:

configure the locale (like en_US, en_GB, fr_FR), in the directory and use that in CalendarGuiImpl.locale

I'm OK with this. Please name the entry something that is clearly associated with the calendar control.

I've added a calendarLocale setting which appears on the same parent as dateFormat (.e.g /server/default):

            <node class="string" name="calendarLocale">
              <node-attribute name="value" value="en_GB"/>
            </node>

If is missing, the default (JVM) locale will be used.

There are two calendar OCX event procedures which are missing in FWD:

PROCEDURE CtrlFrame.DTPicker.Change .
PROCEDURE CtrlFrame.DTPicker.KeyPress .

Sergey, did you encounter these before? Any idea how I can get a list of all available procedures?

Constantin Asofiei wrote:

There are two calendar OCX event procedures which are missing in FWD:
[...]

Sergey, did you encounter these before? Any idea how I can get a list of all available procedures?

No, did you mean list of OCX events for date picker control IDTPicker? CHANGE event should be implemented for CalendarGuiImpl

   /**
    * Fires a new value changed event.
    */
   public void fireValueChanged()
   {
      // honor any Progress triggers
      KeyInput vchng = new KeyInput(this,
                                    EventType.KEY_PRESSED,
                                    Keyboard.SE_VALUE_CHANGED,
                                    true);
      EventManager.postEvent(vchng);
   }

Sergey Ivanovskiy wrote:

Constantin Asofiei wrote:

There are two calendar OCX event procedures which are missing in FWD:
[...]

Sergey, did you encounter these before? Any idea how I can get a list of all available procedures?

No, did you mean list of OCX events for date picker control IDTPicker? CHANGE event should be implemented for CalendarGuiImpl

Isn't this a 4GL event? Even so, I don't think you can write this in 4GL via some UI trigger.

Here I'm talking about OCX events which are managed by 'OCX event procedures' with a specific name, in 4GL.

In the AppBuilder, after pressing CTRL-S, you can add OCX events for any OCX control. This includes mouse, keys, change, and more.

I'm focusing on the two above now (Change and KeyPress) for the DTPicker.

I've followed OCX Event Procedure Support and I was able to emulate Change and KeyPress COM events easily.

1. in the OCX dtpicker control, if you left-click on the 'month' in the title, you will get a popup to select another month.
2. in the OCX dtpicker control, if you left-click on the 'year' in the title, it will enable up/down buttons to go through the years. Clicking again dismisses the buttons.

These two will automatically update the displayed dates in the middle part.

• by changing the i.e. day via inserting numeric keys
• by using the up/down keys which will scroll through the possible values

This applies to any kind of format.

Some of the look&feel calendar issues (like #4174-226) were fixed in 3821c rev 11858. Also, added 'Change' and 'KeyPress' COM events.

Constantin Asofiei wrote:

Sergey Ivanovskiy wrote:

Constantin Asofiei wrote:

There are two calendar OCX event procedures which are missing in FWD:
[...]

Sergey, did you encounter these before? Any idea how I can get a list of all available procedures?

No, did you mean list of OCX events for date picker control IDTPicker? CHANGE event should be implemented for CalendarGuiImpl

Isn't this a 4GL event? Even so, I don't think you can write this in 4GL via some UI trigger.

Here I'm talking about OCX events which are managed by 'OCX event procedures' with a specific name, in 4GL.

It was told to me to emulate OCX events so it requires rewriting OCX event procedure. Thus I am aware of these issues.

• cursor keys (up/down/left/right)
• page-up/down keys (including with ctrl modifier)
• end/home keys