FWD » Core Development » User Interface

Code review 10706a rev. 16320.

This solution does not work for datetime and datetime-tz.

Run this example:

DEFINE VARIABLE f1 AS DATE.
DEFINE VARIABLE f2 AS DATETIME.
DEFINE VARIABLE f3 AS DATETIME-TZ.
DEFINE VARIABLE counter AS INTEGER NO-UNDO.

DEFINE FRAME fMain f1 f2 f3.
ON VALUE-CHANGED OF f1, f2, f3 DO:
  MESSAGE counter "Value-changed".
  counter = counter + 1.
END.

ENABLE ALL WITH FRAME fMain.
WAIT-FOR WINDOW-CLOSE OF CURRENT-WINDOW.

Pressing Backspace in the f2 and f3 FILL-IN s still fires the VALUE-CHANGED event.

The problem is in how the isDateBased is assigned:

      isDateBased = config.dataType.equalsIgnoreCase("date");

Vladimir Tsichevski wrote:

Code review 10706a rev. 16320.

The problem is in how the isDateBased is assigned:

[...]

What do you suggest? To consider a DATETIME/DATETIME-TZ FILL-IN also a date-based widget, or we need a new variable to store this?

Delia Mitric wrote:

Vladimir Tsichevski wrote:

Code review 10706a rev. 16320.

The problem is in how the isDateBased is assigned:

[...]

What do you suggest? To consider a DATETIME/DATETIME-TZ FILL-IN also a date-based widget, or we need a new variable to store this?

The first variant. Check that setting isDateBased to true for date subclasses will not cause regressions.

The application crashes when we using a DATETIME-TZ FILL-IN and trying to introduce any characters.
I've checked this with the latest trunk revision.

Delia Mitric wrote:

The application crashes when we using a DATETIME-TZ FILL-IN and trying to introduce any characters.
I've checked this with the latest trunk revision.

Confirmed. I will create a separate task for the new issue and take ownership of it.

Note that the proposed change does not fully resolve the original problem for datetime and datetime-tz FILL-IN widgets.

When testing the BACKSPACE handling in these types, avoid entering any digits to prevent interference from the new issue.

Vladimir Tsichevski wrote:

Delia Mitric wrote:

The application crashes when we using a DATETIME-TZ FILL-IN and trying to introduce any characters.
I've checked this with the latest trunk revision.

Confirmed. I will create a separate task for the new issue and take ownership of it.

I've created #11053 for this issue.

Vladimir, when you get a chance, please review this one. Thank you!

Vladimir Tsichevski wrote:

Delia Mitric wrote:

Vladimir, when you get a chance, please review this one. Thank you!

10706a rev. 16321 code review and testing.

The change is good, it fixes the original review for date, datetime and datetime-tz.

I would optimize the change a bit and get rid of additional FillIn field isDateBased.
See 10706-optimize.diff.

Rebased the branch to the latest trunk revision and I applied your suggestions to optimize the changes.
Please check out 16371 revision.

10706a rev. 16371 – Code review & testing¶

Code review¶

1. FillIn.java and FillInGuiImpl.java: newly added history entry lines exceed the 110-character limit.
2. Questionable caret positioning logic after selection handling:
   

   // Move cursor at the most left position of the selection
   setCursorOffset(config.selectionStart + 1);
   

   
   This sets the caret one character right of the selection start, relying on subsequent code to correct it.

Proposed cleaner replacement (please, check if this is safe):

// Move cursor at the most left position of the selection
setCursorOffset(config.selectionStart);
invalidateSelection();
return;

Testing results – fixed original issue, new issues found¶

The proposed change eliminates the unwanted VALUE-CHANGED on plain BACKSPACE.

However, the following related inconsistencies were observed:

1. Ctrl-Backspace with text selection
   • OE: behaves like plain Backspace — deletes selection, places caret at left boundary of former selection.
   • FWD: caret placement depends on selection direction:
     • Right-to-left selection → caret at left boundary (matches OE)
     • Left-to-right selection → caret at right boundary (incorrect)
2. Delete key when exactly one delimiter character is selected
   • OE: does nothing (no change, no event)
   • FWD: deselects the character, moves caret, and triggers VALUE-CHANGED event (incorrect)
3. Ctrl-Delete with selection
   • OE: deletes/replaces selection with spaces, places caret at left boundary (same as plain Delete)
   • FWD: does not delete the selection, places caret at left or right boundary (behaves like current FWD Ctrl-Backspace)
4. DATETIME (and likely DATETIME-TZ) FILL-IN with unknown screen value:
   When pressing Space, . (dot) or ; (semicolon):
   • OE: ignores the keys (no change, no delimiters shown)
   • FWD: displays all format delimiters

   Note: DATETIME-TZ currently crashes due to #11053, but would likely show the same delimiter-population behaviour if that crash were fixed.