Bug 495352 - [Tooling] Improve resize handling of parameter table in properties view for protocol message
Summary: [Tooling] Improve resize handling of parameter table in properties view for p...
Status: CLOSED FIXED
Alias: None
Product: Papyrus-rt
Classification: Modeling
Component: tool (show other bugs)
Version: 0.7.2   Edit
Hardware: PC Windows 7
: P4 normal
Target Milestone: 0.9.0   Edit
Assignee: Young-Soo Roh CLA
QA Contact:
URL:
Whiteboard:
Keywords: ui, usability
Depends on: 489720 489728 504077
Blocks:
  Show dependency tree
 
Reported: 2016-06-03 02:22 EDT by Peter Cigehn CLA
Modified: 2016-12-15 03:30 EST (History)
5 users (show)

See Also:


Attachments
UMLRT properties tab (59.90 KB, image/jpeg)
2016-12-14 10:07 EST, Young-Soo Roh CLA
no flags Details
UML properties tab (124.05 KB, image/jpeg)
2016-12-14 10:07 EST, Young-Soo Roh CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Cigehn CLA 2016-06-03 02:22:00 EDT
There are issues related to resize handling of the parameter table in the properties view for protocol messages that needs to be improved. These issues was originally identified as part of Bug 476984. The remaining issues from that Bugzilla have been broken out into separate Bugzillas, and this one is for tracking these remaining resize issues.

Some of the issues that can be seen are:

1) The parameter table have a fixed size, causing a horizontal scrollbar to appear if the properties view is too narrow, or if the properties view is wide enough, the table does not fill the remaining space as expected.
2) When resizing the properties view, the size of the parameter table is not adjusted accordingly either, since it seem to be of fixed size.
3) When adding additional parameters to the table, a vertical scrollbar appears on the table, instead of simply making the table bigger. When switching back and forth to some other element, and selecting the same protocol message again makes the table appear without that horizontal scrollbar as expected. This resizing should be done already when new parameters are added to the table.

Some of these aspects is very much related also to the issue identified in Bug 495146.
Comment 1 Simon Redding CLA 2016-09-09 14:15:53 EDT
Does not gate v1.0
Comment 2 Charles Rivet CLA 2016-09-09 14:53:12 EDT
Added "ui, usability" keywords. I am thinking we will need a "UX" bug clearance after 1.0 is out to fix this and other usability issues that already have bugs, in addition to those our users will discover.

With our current process, this would not fit in an MVP, but it is needed.
Comment 3 Peter Cigehn CLA 2016-10-03 05:27:13 EDT
I would really like to move this 0.8 and that we take care of this one at the same time as Bug 495146. The first attempt at fixing Bug 495146 causes a regression in the same table for the properties view. Since they seem to be so related (and apparently share the same code), I have a strong feeling that the best is to fix these two together at the same time.

Please move this from "Future" to "0.8" (or move the other one to future as well). They should really be implemented together.
Comment 4 Young-Soo Roh CLA 2016-10-06 10:13:21 EDT
issues 1) and 2) depends on bug#504077.
Comment 5 Peter Cigehn CLA 2016-10-06 10:43:21 EDT
Could we plan these improvements for 0.9, instead of Future? Now when we have worked with this so much, and identified the needed fixes in base Papyrus, I really think that we should try to get these improvements into 0.9 (and thus also ensure that the needed fixes in base Papyrus gets into Neon.2).
Comment 6 Peter Cigehn CLA 2016-12-06 06:50:13 EST
I just checked the latest Papyrus-RT build (based on the latest Papyrus build where Bug 504077 now have been marked as resolved). From what I can see, all the issues identified in Comment 0 is still there, so I assume that some adaptation is still needed on the Papyrus-RT side to utilize the fixes that have been made in Papyrus.

I suggest that this bug also covers the improvement of the table with triggers for a transition as well, which currently seem to have the same kind of issues.
Comment 7 Eclipse Genie CLA 2016-12-13 14:37:28 EST
New Gerrit change created: https://git.eclipse.org/r/87076
Comment 8 Young-Soo Roh CLA 2016-12-13 14:48:26 EST
(In reply to Eclipse Genie from comment #7)
> New Gerrit change created: https://git.eclipse.org/r/87076

Fix for 1) and 2) from comment0.

Auto resizing vertically when parameter added is not possible. We can make default vertical size bigger if it helps.


There are still some changes from bug#504077 to be merged to 2.0 maintenance stream so tables with odd number of columns will not align perfectly.
Comment 9 Peter Cigehn CLA 2016-12-14 05:33:57 EST
(In reply to Young-Soo Roh from comment #8)
> (In reply to Eclipse Genie from comment #7)
> > New Gerrit change created: https://git.eclipse.org/r/87076
> 
> Fix for 1) and 2) from comment0.
> 
> Auto resizing vertically when parameter added is not possible. We can make
> default vertical size bigger if it helps.
> 
> 
> There are still some changes from bug#504077 to be merged to 2.0 maintenance
> stream so tables with odd number of columns will not align perfectly.

I quickly tested this patch set, and yest issue 1) and 2) seem to be fixed. I am not sure that I can see any issue with odd number of columns, but maybe it is relative small. I guess that whenever that fix is made in Papyrus, we will get it "for free" in Papyrus-RT (meaning that we don't need any code modifications on the Papyrus-RT side).

Regarding issue 3 I am not sure I understand "is not possible". It is software, anything is possible! :) I assume that you mean that it is not possible the current functionality and API provided by the table support from Papyrus? I suggest that we write a separate bug specifically for issue 3 (maybe there is a need for one on Papyrus-RT and one for adding some core support in Papyrus), since I feel that it currently behaves rather inconsistently, i.e. if you add new parameters/triggers then you get the scroll bar. But if you now switch back and forth and get back again, then the table has automatically been extended to show all rows. From a user point of view I find this kind of inconsistent. 

I really do not see that having a bigger vertical default size really helps, since you will always bump into the issue anyway.

Apart from that, the size of the table is always "too big", i.e. if you have added a number of parameters/triggers (that then causes the scroll bar), switch back-and-forth, and get back to when the table have been automatically adjusted, then there are space added for 3-4 extra rows (which also feels redundant). This especially visible when you bring up the protocol message edit dialog (from the protocol message list in the properties view of a protocol).

If you hade the automatic resizing done already when adding new rows, then adding that extra space of 3-4 rows would not be needed.

Any comments regarding "is not possible"?
Comment 10 Young-Soo Roh CLA 2016-12-14 09:23:58 EST
Current behaviour is consistent with other tables and list from UMLRT and UML properties view. Even if we want to change this behaviour different from the base Papyrus UML properties view it is not possible in the current supported API from SWT as far as I know. You need to call layout for whole page and the properties view is not supposed to work that way. That's why the table grows only when the properties view is redrawn. e.g., click somewhere else and come back. I am not sure if anyone else has different opinion but I don't think we can resize table vertically when new items are added.

(In reply to Peter Cigehn from comment #9)
> (In reply to Young-Soo Roh from comment #8)
> > (In reply to Eclipse Genie from comment #7)
> > > New Gerrit change created: https://git.eclipse.org/r/87076
> > 
> > Fix for 1) and 2) from comment0.
> > 
> > Auto resizing vertically when parameter added is not possible. We can make
> > default vertical size bigger if it helps.
> > 
> > 
> > There are still some changes from bug#504077 to be merged to 2.0 maintenance
> > stream so tables with odd number of columns will not align perfectly.
> 
> I quickly tested this patch set, and yest issue 1) and 2) seem to be fixed.
> I am not sure that I can see any issue with odd number of columns, but maybe
> it is relative small. I guess that whenever that fix is made in Papyrus, we
> will get it "for free" in Papyrus-RT (meaning that we don't need any code
> modifications on the Papyrus-RT side).
> 
> Regarding issue 3 I am not sure I understand "is not possible". It is
> software, anything is possible! :) I assume that you mean that it is not
> possible the current functionality and API provided by the table support
> from Papyrus? I suggest that we write a separate bug specifically for issue
> 3 (maybe there is a need for one on Papyrus-RT and one for adding some core
> support in Papyrus), since I feel that it currently behaves rather
> inconsistently, i.e. if you add new parameters/triggers then you get the
> scroll bar. But if you now switch back and forth and get back again, then
> the table has automatically been extended to show all rows. From a user
> point of view I find this kind of inconsistent. 
> 
> I really do not see that having a bigger vertical default size really helps,
> since you will always bump into the issue anyway.
> 
> Apart from that, the size of the table is always "too big", i.e. if you have
> added a number of parameters/triggers (that then causes the scroll bar),
> switch back-and-forth, and get back to when the table have been
> automatically adjusted, then there are space added for 3-4 extra rows (which
> also feels redundant). This especially visible when you bring up the
> protocol message edit dialog (from the protocol message list in the
> properties view of a protocol).
> 
> If you hade the automatic resizing done already when adding new rows, then
> adding that extra space of 3-4 rows would not be needed.
> 
> Any comments regarding "is not possible"?
Comment 11 Peter Cigehn CLA 2016-12-14 09:59:09 EST
(In reply to Young-Soo Roh from comment #10)
> Current behaviour is consistent with other tables and list from UMLRT and
> UML properties view. Even if we want to change this behaviour different from
> the base Papyrus UML properties view it is not possible in the current
> supported API from SWT as far as I know. You need to call layout for whole
> page and the properties view is not supposed to work that way. That's why
> the table grows only when the properties view is redrawn. e.g., click
> somewhere else and come back. I am not sure if anyone else has different
> opinion but I don't think we can resize table vertically when new items are
> added.
> 

In what other places in base Papyrus is this kind of table being used in the properties view (and edit dialogs)? From what I understood when it was originally introduced, that Papyrus-RT was about to one of the first usages of the tables in this way (at least if I understood Camille when he originally proposed this). Hence also the reason why we it is Papyrus-RT that have found all these limitations.

When comparing with the the ordinary lists, e.g. if you just look at the trigger list on the standard UML tab, then this list has indeed a fixed size and does not get bigger when adding more. But then it does not resize then next time you activate the properties view. The difference here is that the table (seemingly inconsistently) resizes after switch to another element and back, whereas the ordinary lists have a truly fixed size. That is what I personally find confusing here. 

Could you give me another example of how some table/list behaves like this in base Papyrus?

But if you say that this is technically how the properties view have been designed, then I guess it is not much to do about it.

One big difference that I can see when I compare this with the legacy tooling is that all tables similar to this one, are all contained in their own tab, always filling the complete properties view when that tab is selected, e.g. the trigger table is contained in its own trigger tab in the properties view for a transition. So this makes this extending scenario to be quite different, since the table is always the same size as the properties view itself. So this technical issue that you describe is not at all equally visible. Ant those tables are not used in edit dialogs as we have in Papyrus either.

Even though I find the current behavior a bit confusing (especially this also that whenever the table to get resized, when switching back and forth from another element) that it now has 3-4 empty rows (even more empty rows the more rows the table already have) at the bottom (which is especially visible also in the protocol message edit dialog), I guess I have to rest my case (especially if it is only me that gets annoyed by these things).

This resizing of the table will also have an impact on the current layout of the UML-RT tab for the transitions, since the guard and effect properties are placed at the bottom, below the table. Which means that if the transition have lots of triggers (for some reason), the table will extend and now move these properties out of vision (on the other hand this will be resolved anyway when  Bug 507965 is fixed), but it is then just an example of how careful we need to be when mixing tables with other properties on the same tab. Maybe we need to go in the same direction as the legacy tooling and always place these kinds of tables on an own tab (or sub-tab of the UML-RT tab similar to how the Language sub-tab on the root element of a model).
Comment 12 Young-Soo Roh CLA 2016-12-14 10:07:10 EST
Created attachment 265880 [details]
UMLRT properties tab
Comment 13 Young-Soo Roh CLA 2016-12-14 10:07:41 EST
Created attachment 265881 [details]
UML properties tab
Comment 14 Young-Soo Roh CLA 2016-12-14 10:12:36 EST
I added snapshots taken from my runtime workbench. I am not sure why it shows different behaviour from yours. One thing we could do is that we can fix the size of table so that the table will not resize depending on number of items. 

(In reply to Peter Cigehn from comment #11)
> (In reply to Young-Soo Roh from comment #10)
> > Current behaviour is consistent with other tables and list from UMLRT and
> > UML properties view. Even if we want to change this behaviour different from
> > the base Papyrus UML properties view it is not possible in the current
> > supported API from SWT as far as I know. You need to call layout for whole
> > page and the properties view is not supposed to work that way. That's why
> > the table grows only when the properties view is redrawn. e.g., click
> > somewhere else and come back. I am not sure if anyone else has different
> > opinion but I don't think we can resize table vertically when new items are
> > added.
> > 
> 
> In what other places in base Papyrus is this kind of table being used in the
> properties view (and edit dialogs)? From what I understood when it was
> originally introduced, that Papyrus-RT was about to one of the first usages
> of the tables in this way (at least if I understood Camille when he
> originally proposed this). Hence also the reason why we it is Papyrus-RT
> that have found all these limitations.
> 
> When comparing with the the ordinary lists, e.g. if you just look at the
> trigger list on the standard UML tab, then this list has indeed a fixed size
> and does not get bigger when adding more. But then it does not resize then
> next time you activate the properties view. The difference here is that the
> table (seemingly inconsistently) resizes after switch to another element and
> back, whereas the ordinary lists have a truly fixed size. That is what I
> personally find confusing here. 
> 
> Could you give me another example of how some table/list behaves like this
> in base Papyrus?
> 
> But if you say that this is technically how the properties view have been
> designed, then I guess it is not much to do about it.
> 
> One big difference that I can see when I compare this with the legacy
> tooling is that all tables similar to this one, are all contained in their
> own tab, always filling the complete properties view when that tab is
> selected, e.g. the trigger table is contained in its own trigger tab in the
> properties view for a transition. So this makes this extending scenario to
> be quite different, since the table is always the same size as the
> properties view itself. So this technical issue that you describe is not at
> all equally visible. Ant those tables are not used in edit dialogs as we
> have in Papyrus either.
> 
> Even though I find the current behavior a bit confusing (especially this
> also that whenever the table to get resized, when switching back and forth
> from another element) that it now has 3-4 empty rows (even more empty rows
> the more rows the table already have) at the bottom (which is especially
> visible also in the protocol message edit dialog), I guess I have to rest my
> case (especially if it is only me that gets annoyed by these things).
> 
> This resizing of the table will also have an impact on the current layout of
> the UML-RT tab for the transitions, since the guard and effect properties
> are placed at the bottom, below the table. Which means that if the
> transition have lots of triggers (for some reason), the table will extend
> and now move these properties out of vision (on the other hand this will be
> resolved anyway when  Bug 507965 is fixed), but it is then just an example
> of how careful we need to be when mixing tables with other properties on the
> same tab. Maybe we need to go in the same direction as the legacy tooling
> and always place these kinds of tables on an own tab (or sub-tab of the
> UML-RT tab similar to how the Language sub-tab on the root element of a
> model).
Comment 15 Peter Cigehn CLA 2016-12-14 10:58:50 EST
(In reply to Young-Soo Roh from comment #14)
> I added snapshots taken from my runtime workbench. I am not sure why it
> shows different behaviour from yours. One thing we could do is that we can
> fix the size of table so that the table will not resize depending on number
> of items. 

Originally, I was referring to the trigger list on the UML tab for a transition. That list at least seemed to have a "fixed" size when I tested this initially, even when switching back-and-forth to another element. But now when I test it again, it do get resized.

One difference I can see though is that the lists only get one additional row when the get resize after switching back-and-forth, whereas the tables seem to get this progressively more empty rows, i.e. the more rows the table have, the more empty rows are added at the bottom, which will cause them to take up unnecessary much space when they grow larger.

So with that, maybe the conclusion that the core issue I can see is that the tables, gets unnecessary many empty rows when they get resized, compared to the ordinary lists which consistently only get one additional row (disregarding how many rows they contain). 

Since the tables seem to behave consistently with how the ordinary lists behaves, also considering the technical aspects with SWT you mention, then we just to find a reasonable default size of both lists and tables, and then ensure that they behave the same regarding how many additional empty rows they have. I guess this then is an issue in base Papyrus.
Comment 16 Young-Soo Roh CLA 2016-12-14 11:13:40 EST
I see that extra empty spaces are added below the table when table is resized. This table is drawn from o.e.p.uml.properties.widgest.NattablePropertyEditor and should be addressed from the base papyrus.

(In reply to Peter Cigehn from comment #15)
> (In reply to Young-Soo Roh from comment #14)
> > I added snapshots taken from my runtime workbench. I am not sure why it
> > shows different behaviour from yours. One thing we could do is that we can
> > fix the size of table so that the table will not resize depending on number
> > of items. 
> 
> Originally, I was referring to the trigger list on the UML tab for a
> transition. That list at least seemed to have a "fixed" size when I tested
> this initially, even when switching back-and-forth to another element. But
> now when I test it again, it do get resized.
> 
> One difference I can see though is that the lists only get one additional
> row when the get resize after switching back-and-forth, whereas the tables
> seem to get this progressively more empty rows, i.e. the more rows the table
> have, the more empty rows are added at the bottom, which will cause them to
> take up unnecessary much space when they grow larger.
> 
> So with that, maybe the conclusion that the core issue I can see is that the
> tables, gets unnecessary many empty rows when they get resized, compared to
> the ordinary lists which consistently only get one additional row
> (disregarding how many rows they contain). 
> 
> Since the tables seem to behave consistently with how the ordinary lists
> behaves, also considering the technical aspects with SWT you mention, then
> we just to find a reasonable default size of both lists and tables, and then
> ensure that they behave the same regarding how many additional empty rows
> they have. I guess this then is an issue in base Papyrus.
Comment 17 Peter Cigehn CLA 2016-12-14 11:19:37 EST
(In reply to Young-Soo Roh from comment #16)
> I see that extra empty spaces are added below the table when table is
> resized. This table is drawn from
> o.e.p.uml.properties.widgest.NattablePropertyEditor and should be addressed
> from the base papyrus.
> 

Do you write a bug on Papyrus regarding this issue (since you know the details)? Maybe it is best also to have a corresponding tracking bug on the Papyrus-RT side (even though I guess we don't need to adapt anything on the Papyrus-RT side when the fix has been made in Papyrus) so that we can keep track and follow-up on the Papyrus-RT side?

When we have those two bugs in place, then I guess that when the Gerrit change have been merged, we can consider this one to be resolved fixed.
Comment 18 Young-Soo Roh CLA 2016-12-14 13:43:38 EST
bug#509245 and bug#509243 raised.
Comment 20 Peter Cigehn CLA 2016-12-15 03:30:09 EST
I have tested this in the latest Papyrus-RT build. Issue 1) and 2) are now fixed and the table fills the width of the properties view without superfluous horizontal scrollbars appearing. Resizing the properties view (and the protocol message edit dialog) also resizes the table horizontally as expected.

The vertical resize as indicated in 3) is concluded that the current behavior is aligned with how the ordinary lists in the Papyrus properties view behaves. The difference in how additional, empty, rows are added to the table is tracked separately in Bug 509245 (which depends on Bug 509243 in Papyrus).

I put this one into verified fixed.
Comment 21 Peter Cigehn CLA 2016-12-15 03:30:34 EST
Closing as verified fixed.