Project

General

Profile

Bug #2921

Bug #2677: fix drawing and functional differences between P2J GUI and 4GL GUI

fix vertical placement of FILL-IN (editable or not)/LABEL/EDITOR text

Added by Constantin Asofiei over 8 years ago. Updated about 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Eugenie Lyzenko
Target version:
FWD - Cleanup and Stabilization for GUI
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:

text_vert_location_en_4gl.jpg - 4GL enabled widgets (68.3 KB) Eugenie Lyzenko, 04/05/2019 05:59 PM

text_vert_location_dis_4gl.jpg - 4GL disabled widgets (77 KB) Eugenie Lyzenko, 04/05/2019 05:59 PM

text_vert_location_en_fwd_web_3811a_20190405a.jpg - FWD enabled (54.9 KB) Eugenie Lyzenko, 04/05/2019 07:26 PM

text_vert_location_dis_fwd_web_3811a_20190405b.jpg - FWD disabled (54.2 KB) Eugenie Lyzenko, 04/05/2019 07:26 PM

2921_text_vert_location_en_fwd_20190408a.jpg - Web client FWD enabled fix (57.1 KB) Eugenie Lyzenko, 04/08/2019 10:37 AM

2921_text_vert_location_dis_fwd_20190408b.jpg - Web client FWD disabled fix (57.2 KB) Eugenie Lyzenko, 04/08/2019 10:38 AM

History

#1 Updated by Constantin Asofiei over 8 years ago

From #2704 note 12:

Vertical placement is still off by a few pixels... this is mostly related to drawing the text centered vertically. I think instead ascent/descent values need to be used (especially when editable mode is on).

Vertical placement of text in GUI is not 100% accurate in Swing, and in Web client the vertical text is not centered correctly - we might need to compute ascent/descent values for Web (or other means to center the text correctly).

#2 Updated by Greg Shah about 8 years ago

  • Target version changed from Milestone 12 to Milestone 16

#3 Updated by Greg Shah over 7 years ago

Several other widgets are affected too, including buttons, radio-buttons and toggle-box (maybe this is already considered with labels).

#4 Updated by Greg Shah over 7 years ago

See #3054-151 through #3054-161 for some examples in both web and swing.

#5 Updated by Greg Shah over 7 years ago

  • Target version changed from Milestone 16 to Cleanup and Stabilization for GUI

#6 Updated by Greg Shah about 5 years ago

  • Assignee set to Eugenie Lyzenko

#7 Updated by Eugenie Lyzenko about 5 years ago

Starting to work on the issue. What are the planes for changes to apply? Create separate branch or I can upload to current 3811a branch?

If separate one, should I start the regression testing for 3811a and merge into trunk in case of no regressions found?

#8 Updated by Greg Shah about 5 years ago

Use 3811a.

We are first trying to get 3750b into trunk ASAP. We aren't changing trunk until then.

#9 Updated by Eugenie Lyzenko about 5 years ago

Greg Shah wrote:

Use 3811a.

We are first trying to get 3750b into trunk ASAP. We aren't changing trunk until then.

OK. Understood.

#10 Updated by Eugenie Lyzenko about 5 years ago

Two samples from 4GL to compare vertical text location:

Enabled widgets:

4GL enabled widgets

Disabled widgets:

4GL disabled widgets

#11 Updated by Eugenie Lyzenko about 5 years ago

Deviations found.

Swing:
  • disabled Fill-In text location is higher than expected(1 pixel)
Web, widgets enabled:
  • enabled Fill-In text location is higher than expected(1 pixel)
  • Label for Fill-In is higher than expected(1 pixel)
  • enabled Editor text location is higher than expected(1 pixel)
  • Label for Editor is higher than expected(1 pixel)
  • Static text and label for it is higher than expected(1 pixel)
  • Label for Toggle-box is higher than expected(1 pixel)
  • Label and cell text for Browse is higher than expected(1 pixel)
  • Label for Radio-set is higher than expected(1 pixel)
  • Label for Button is higher than expected(1 pixel)
Web, widgets disabled(in addition to issues found for enabled state):
  • disabled Fill-In text location is higher than expected(2 pixels)

The pictures for FWD Web client:

Enabled widgets:

FWD enabled

Disabled widgets:

FWD disabled

Excluding disabled fill-in content 1 pixel shift(in 4GL the fill-in disabling should shift the text 1 pixel down) the others deviations can be also explained by text height mismatch for Web client. For the same font web client text is one pixel smaller. If we compute text Y coordinate based on text height this can explain total 1 pixel wrong shift. This is the GUI driver dependent issue. While disabled Fill-in has GUI driver independent issue. Both types will be fixed separately, independent will be first.

#12 Updated by Eugenie Lyzenko about 5 years ago

Task branch 3811a has been updated for review to revision 11318.

This is the fix for GUI driver independent vertical text placement for disabled FILL-IN widget issue. The text in entry field need to be shifted 2 pixels down comparing to enabled.

The next part to fix is GUI dependent issue with text 1 pixel placement/height deviation for GUI Web driver. Need to investigate if the our text height/vertical starting point is fine for Web driver JS code. Continue working.

#13 Updated by Eugenie Lyzenko about 5 years ago

Task branch 3811a has been updated for review to revision 11320.

This is the fix for GUI Web driver text vertical placement mismatch. Also I have found for some widgets FWD underlines the first letter of the label when it should not. Also fixed. So the pictures in FWD are now:

Web client, enabled widgets.

Web client FWD enabled fix

Web client, disabled widgets.

Web client FWD disabled fix

#14 Updated by Eugenie Lyzenko about 5 years ago

  • Status changed from WIP to Review
  • % Done changed from 0 to 100

Looks completed, need review and probably more screen to check the consistency.

#15 Updated by Greg Shah about 5 years ago

Code Review Task Branch 3811a Revision 11320

The changes look quite good. Some feedback:

1. I would like common code for the implementation of the edit*() methods since the only implementation difference is related to the widget type in the error message. I see that we can't use default methods of SelectableText because of the lack of backing for getId() and the type name. And I definitely don't want this code in GenericWidget. How about moving the implementation to a clipboard helper class?

2. By convention, message types sent from JS UP to Java (like MSG_CLIPBOARD_SOFT_PASTE_READY) should be in the "lower range" of the codes where the most significant bit is 0 (0x00 instead of 0x80). The Java to JS message types are kept in the upper range.

3. CommonWidget needs a history entry.

Hynek/Constantin: Please review.

#16 Updated by Eugenie Lyzenko about 5 years ago

Greg Shah wrote:

Code Review Task Branch 3811a Revision 11320

The changes look quite good. Some feedback:

1. I would like common code for the implementation of the edit*() methods since the only implementation difference is related to the widget type in the error message. I see that we can't use default methods of SelectableText because of the lack of backing for getId() and the type name. And I definitely don't want this code in GenericWidget. How about moving the implementation to a clipboard helper class?

OK. I'll upload reworked code later.

2. By convention, message types sent from JS UP to Java (like MSG_CLIPBOARD_SOFT_PASTE_READY) should be in the "lower range" of the codes where the most significant bit is 0 (0x00 instead of 0x80). The Java to JS message types are kept in the upper range.

3. CommonWidget needs a history entry.

Fixed.

Task branch 3811a has been updated for review to revision 11322.

#17 Updated by Eugenie Lyzenko about 5 years ago

Greg Shah wrote:

Code Review Task Branch 3811a Revision 11320

The changes look quite good. Some feedback:

1. I would like common code for the implementation of the edit*() methods since the only implementation difference is related to the widget type in the error message. I see that we can't use default methods of SelectableText because of the lack of backing for getId() and the type name. And I definitely don't want this code in GenericWidget. How about moving the implementation to a clipboard helper class?

Planning to introduce new class p2j/ui/ClipboardHelper. The currently existed p2j/util/ClipboardManager is for some other goals comparing to what we need here.

#18 Updated by Eugenie Lyzenko about 5 years ago

Greg Shah wrote:

Code Review Task Branch 3811a Revision 11320

The changes look quite good. Some feedback:

1. I would like common code for the implementation of the edit*() methods since the only implementation difference is related to the widget type in the error message. I see that we can't use default methods of SelectableText because of the lack of backing for getId() and the type name. And I definitely don't want this code in GenericWidget. How about moving the implementation to a clipboard helper class?

Implemented in 3811a revision 11323.

#19 Updated by Eugenie Lyzenko about 5 years ago

  • Status changed from Review to Test

As part of the 3811a the changes passed regression testing. So it is in good shape for further rebase/commit when necessary.

#20 Updated by Greg Shah about 5 years ago

  • Status changed from Test to Closed

Branch 3811a was merged to trunk as revision 11303.

Also available in: Atom PDF