FWD » Core Development » User Interface

Code Review 0923a

1. In WindowGuiImpl do we need to keep the hard coded message output (e.g. line 1...)? Even with the limited GUI client support it seems that a message statement on the default window could be made to work. If not, what are the specific inhibitors? The obvious thing missing is that MessageAreaImpl is not connected.

2. If I understand correctly, the full list processing of the message area is not there yet. Have you answered the list processing questions noted in #2252 notes 79 and 212? You can go further with the list processing in this first pass without other GUI support, right?

3. What is the idea behind ScrollBar.Bars.BOTH? It seems like it would be easier to implement separate instances (one HORIZONTAL and one VERTICAL) than to implement a single instance that draws in two places.

4. I think it makes sense to explain in javadoc that for VERTICAL scrollbars, ScrollBar.Position.LEFT is the "up" and ScrollBar.Position.RIGHT is "down".

5. Why not put the scroll listener notification code into ScrollBar instead of ScrollBarGuiImpl?

6. MessageLineGui needs a history entry.

7. ScrollableListGuiImpl and MessageAreaGuiImpl need standard headers and class javadoc.

1. The implementation of GUI widgets still depends on coordinate conversion between pixels units and character units.

Yes, understood.

2. Mouse and keyboard support should be added to widgets.

Yes.

3. Unfortunately at this point we cannot test the GUI client because is not yet properly implemented.

But at this point we can at least get the STATUS and MESSAGE statements working with the DEFAULT-WINDOW. Or is there something broken that doesn't allow that?

I am suggesting that you would not use ask.p since that requires frame support. But you can run some very simple code like:

MESSAGE "Hello World!".

Message and status statements don't rely upon frames or other widgets. So long as the ThinClient methods are connected to the backing GUI implementation it should work.

Code Review 0925a

This is really good! I'm excited that you have achieved the "Hello World" for GUI.

My only comment is that I see at least 2 pieces of code that will duplicated in every widget, but which probably could be common code: repaint() and the draw() code that detects if the font has changed and forces a relayout of the containing window. There may be more code than this, but it seems like all widgets will have this in common. How do we most effectively move such code to a common location when all GUI widgets don't have a single common GUI-specific parent (their common parents are not specific to ChUI or GUI)? GuiPrimitives is too low-level for that code and GuiScreenDriver is too far away from widget-level concerns. Perhaps GuiOutputManager is a good location?

Or can the common features be refactored to be truly common to both ChUI and GUI (although the font changing logic definitely only affects GUI)?

Yes we could do the code more abstract. But first of all I want to have the coordinates fixed.
Fonts are used only on GUI clients because FONT-TABLE and COLOR-TABLE are available only for GUI interfaces.
Also both fonts and colors could be changed dynamically and should be reevaluate whenever the widgets are draw.
For efficiency here we have first to check if they are really changed and reevaluate if and only if has been changed.

Agreed and understood.

Should be the same value as for ChUI or should be a parameter stored in server directory?

As I noted in note 79 of #2252, I previously found no way to change the number of message lines, whether GUI or ChUI. There is no reason to have this be parameterized, when Progress itself hard codes it. Just fix this as the same value for both GUI and ChUI.

The mouse enabled should be a parameter in WindowConfig? Who decides if a window has mouse or not?

Have you found a configurable way for the 4GL developer to specify this under program control? If not, then this is OK to do as part of the GUI-specific code. Although we do know that there is some limited possible ChUI mouse support, we have not yet seen a customer that actually uses it. So for now, having the GUI code enable mouse is fine.

The algorithm works fine but not for WindowTitleBar which is not yet properly implemented is just a dummy implementation. I think that on the next task we have to provide an appropriate implementation for WindowTitleBar which shall use the same algorithm to find mouse sources.

Understood. This will have to be part of #2340. Please add an entry to that task to explain this requirement.

Another issue that should be fixed here is to implement an algorithm to select the right widget when widgets are overlapped perhaps using the Z order.

Yes. This probably needs to be done as part of #2370. Please add an entry to that task to explain this requirement.

Regarding mouse click I think that we have to add more code on handlers like which button was clicked, if double click etc.

I'm not sure about this. When it gets to the 4GL widget mouse-specific behavior, we will start to need the 4GL event system to be mouse-aware. In other words, double clicking, choose and other 4GL events can be generated by mouse processing and this common support is probably not going to be implemented in widget-specific handlers.

What should be still fixed is to set a minimum size for thumb button. The size of thumb button is calculated and this size could be result small even zero so we have define a minimum size.

OK. Please do fix this.

I inherited as much as possible from the current ChUI widgets implementation. I think we need a mechanism to communicate between widgets. I know that we already have an event based system which allow to send notifications to registered listeners but my idea is to design a direct communication mechanism like a direct call for example: widget.sendMessage(Message message)

I don't understand the requirement here. I don't know of any case where one widget needs to communicate to another widget. For the MESSAGE statement, the input will be coming from the server, through ThinClient and then passed to the containing Window. That window instance would be responsible for routing it to the contained message area. Please see Vadim's work in #2229 where he is very far along in implementing this for the MESSAGE statement.

Do you have other examples besides MESSAGE or STATUS? Both of these would be handled the same way. Is there some general case of widget to widget communication?

Even thought the widgets will suffer more changes in the future the message area could be considered actually as implemented. I think that we have to keep the same paradigm when we have to create new widgets. A complex widgets should be build from basic widgets. For example the WindowTitleBar should be designed as a container having an icon, a system menu and 3 buttons. Also system menu should be designed as a pop-up menu which in turn is a specialized menu widget. Menu widgets are containers containing menu items.

I generally agree with this. However, we need to be very clear in the boundary between a 4GL widget and these sub-widget components.

Why do the message-area5.png, message-area6.png and message-area7.png show a message area that is 8 lines in size? The 4GL GUI doesn't change size like that as far as I know. The list is always only 2 lines tall, the scrollbar never has a thumb and the list is scrolled line by line using the up/down arrow buttons. Were your examples just for testing?

The picture are uploaded just to show how scroll bar looks. I forced 8 lines instead 2 for message area.
When message area has only 2 visible lines the scroll bar is displayed only when the number of lines stored on message area is greater than 2 and only buttons are displayed like in message-area4.png because the height of the message area does not allow drawing other parts of scroll bar.

Code Review 0930a

This looks really good.

1. There are still some hard coded message lines left behind.

2. If I understand correctly, the list management features are not yet compatible with the 4GL. In other words, there seems to be no max list size and the dropping of old messages is not implemented.

3. Please post answers to each of the open questions listed in note 79 of #2252. I would like to make sure that we fully understand how this should work.

4. Does the MessageAreaGuiImpl.split() method exactly match the splitting behavior of the 4GL?

Greg/Marius: in case you are not aware, see note 3 from #2252 - the message area in GUI holds a maximum of 50 messages. At the time I was investigating this, I couldn't find a way to configure this value.

Thanks Constantin I will check on customer's server.
I have another question related to fonts please: The call to FontManager.getTextWidth require font/text metrics.
I saw 2 files in my p2j project font-metrics.dtd and text-metrics.dtd but no xml files.
Where I could find examples on how to use font and text metrics on my p2j project?

I'm trying to test the following p4 statement current-window:message-area-font=5. which in turn is converted in Java as currentWindow().unwrapWindow().setMessageAreaFont(new integer(5)); but it does not work for GUI clients.
Doing debugs I found why. When the window configuration, is pushed down to client WindowConfig.applyConfig is called but here is the problem:

I'm struggle to understand this snippet of code from WindowConfig class

   public void applyConfig(ComponentConfig config)
   {
      // a hack to not override client-side settings, for GUI mode
      if (OutputManager.getDriver().isChui())
      {
         super.applyConfig(config);

         if (config instanceof WindowConfig)
         {
            WindowConfig other = (WindowConfig) config;

            setStatusAreaFont(other.statusAreaFont);
            setMessageAreaFont(other.messageAreaFont);
            setHelp(other.help);
            setTitleFont(other.getTitleFont());

            // TODO: which other data needs to be re-applied?
         }
      }
      else
      {
         setTitle(((WindowConfig) config).getTitle());
      }
   }

In class history I found: Added GUI-only hack to allow the window title to be pushed from the server-side to client-side, without overriding other config.
But: setMessageAreaFont(other.messageAreaFont) for example should works on GUI only and doesn't work because the condition to be a ChUI client.
What is in fact the logic that should be implemented here?

I'm sure that any such code is just temporary and has no deep meaning. You can remove the conditional "protection" logic and see what happens.

Code Review 1001a

The code changes look good.

I think the ThinClient.splitMessage() should probably be moved into the ChUI message impl. Let the TC code be common and delegate the different splitting algorithms to the ChUI/GUI specific widget classes.

Constantin will have to answer the font questions, but generally the font configuration must be captured from the original Windows system. Then we load it at runtime and use those metrics to duplicate the 4GL rendering.

Code Review 1006a

Everything looks good except there is some hard coded message output in MessageAreaImpl.draw().

Currently I'm thinking for a solution to move ThinClient.splitMessage() and make TC code common. Unfortunately much of the code inside ThinClient is ChUI specific which make the separation a little bit more difficult.

Did you look at Vadim's work in #2229? As part of that work, he has moved the splitMessage() implementation into Window. Make sure you don't duplicate his work. The vig_upd20141006a.zip in #2252 has his latest work, but it is not merged with Constantin's changes yet.

Besides this splitMessage() improvement, what else is left to do?

MessageAreaImpl in package com.goldencode.p2j.ui.client.gui is no longer used on GUI. The MessageAreaGuiImpl is used for GUI.
Here is another problem because the MessageAreaGuiImpl cannot extends from MessageArea like MessageAreaImpl and I have to check
for a solution to eliminate the MessageAreaImpl for GUI.

Apart of this problem I think no other works left to do on this task.

Code Review 1007a

client/chui/MessageAreaImpl.java is missing a history entry. Otherwise I am fine with the update. Please start runtime regression testing.

Even if #2415 finds that there is a insert indicator, I don't want to keep MessageLineGui as the widget name that provides such support. It is too confusing since it really seems like this should be a part of the message support. So, any subsequent insert indicator support should probably be named InsertIndicator.java.

When task #2229 will be finished I have to do the appropriate changes on GUI message area implementation.

Understood.

Code Review 1007b

I'm good with the update.