Bug 475905 - [Tooling] The customized icon for UML-RT specific concept shall be used as icon in the properties view as well
Summary: [Tooling] The customized icon for UML-RT specific concept shall be used as ic...
Status: CLOSED FIXED
Alias: None
Product: Papyrus-rt
Classification: Modeling
Component: tool (show other bugs)
Version: .7   Edit
Hardware: PC Windows 7
: P2 normal
Target Milestone: 0.8.0   Edit
Assignee: Christian Damus CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-26 07:22 EDT by Peter Cigehn CLA
Modified: 2016-09-27 08:19 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Cigehn CLA 2015-08-26 07:22:50 EDT
If you select an element in the model explorer which have its own customized icon, e.g. capsule, protocol, protocol message, then the user would expect to see the same icon to be used in the title of the properties view. 

But currently the default icon is shown instead, e.g. for capsule the class icon is shown, for protocol the collaboration icon is shown, for protocol message the operation icon is shown.

It is a bit confusing that the "internal" icon is shown in the properties view, especially for multi-element concepts like protocol and protocol message.

Please make sure that the icon shown in the title of the properties view is the same as the icon being used in the model explorer.
Comment 1 Peter Cigehn CLA 2015-08-28 06:57:06 EDT
In Bug 471826 Comment 25 it was identified that the customized icon for a protocol message shall be used in the the trigger selection dialog. This Bugzilla could be generalized to also include those situations to make sure that in all cases the customized icon is being used. But if this is too general this Bugzilla could be limited to the properties view.
Comment 2 Peter Cigehn CLA 2016-07-19 08:21:33 EDT
As indicated in Bug 496653 Comment 9, the customized capsule icon probably should be used in the header of the capsule structure diagram as well, and not only in the properties view.
Comment 3 Charles Rivet CLA 2016-08-29 13:50:15 EDT
This appears to be true for all Papyrus-RT-specific elements. It also gets worse for protocols as it also displays " <<Protocol>> " before the name.
Comment 4 Christian Damus CLA 2016-08-31 09:19:18 EDT
(In reply to Charles Rivet from comment #3)
> This appears to be true for all Papyrus-RT-specific elements. It also gets
> worse for protocols as it also displays " <<Protocol>> " before the name.

Do we not want to show the UML-RT stereotype keywords in labels, then?  The same would apply to <<Capsule>>, <<RTPort>>, etc. in the Model Explorer view and elsewhere.

I see <<Capsule>> also in the title bar of the Properties view, but not <<RTPort>> nor <<CapsulePart>>.
Comment 5 Peter Cigehn CLA 2016-08-31 09:58:19 EDT
(In reply to Christian W. Damus from comment #4)
> (In reply to Charles Rivet from comment #3)
> > This appears to be true for all Papyrus-RT-specific elements. It also gets
> > worse for protocols as it also displays " <<Protocol>> " before the name.
> 
> Do we not want to show the UML-RT stereotype keywords in labels, then?  The
> same would apply to <<Capsule>>, <<RTPort>>, etc. in the Model Explorer view
> and elsewhere.
> 
> I see <<Capsule>> also in the title bar of the Properties view, but not
> <<RTPort>> nor <<CapsulePart>>.

A recurring comment from users of the legacy tooling, is that don't want to see the stereotypes <<Capsule>>, <<Protocol>> and so on. They have distinct icons and then the stereotype label itself becomes superfluous. At least when you have used the tool for a while. I guess it could still be useful for newcomers to keep the stereotype labels.

But if we get the icon to be consistently be displayed everywhere, then the use of the stereotype label itself I guess quickly becomes superfluous.

Regarding the inconsistency that the stereotype label is not always shown in the title of the properties view I can just comment on that I personally prefer to have it consistently shown. One thin that I myself miss a lot compared with the legacy tooling is the meta-class label (within single < and > as opposed to the stereorype within double << and >>). I never learn all the icons used for all meta-classes in the UML meta-model, so having the meta-class name shown in the title in the properties view I have found to be very useful and something that I miss in Papyrus.

And if we had this kind of "meta-class" label support, we would be able to show this for DSML elements which does not have a stereotype lable itself, e.g. the protocol message which correponds to both an operation and a call event, of which none is stereotyped (a protocol message is implicitly stereotyped by the RTMessageSet stereotype on the Interface owning the Operation).

Not sure if this long-winding discussion really helped... :)

To summarize: Stereotype labels can to some be seen as superfluous, wheraas to other they can be helpful (if you can't memorize the icon). But the stereotype label it not always sufficient, and a meta-class label is also useful especially for elements not explicitly stereotyped and for higer level DSML concepts with their own "meta-classes", e.g. protocol message in UML-RT.
Comment 6 Christian Damus CLA 2016-08-31 10:32:50 EDT
(In reply to Peter Cigehn from comment #5)

Some good points, here, Peter.  In fact, where the metaclass tags in the labels are concerned, we already have a mix in Papyrus-RT (and I suppose in Papyrus):  relationships tend to present these tags, e.g., <Usage>, <PackageImport>.  That would appear to be by delegation to the UML2 generated item providers, whereas for non-relationships, Papyrus has its own label scheme.  I'm not sure.

I generally agree that the keyword for a stereotype that is evident by the icon is redundant.  However, it's not a simple matter to remove those keywords from the labels because the keywords for other stereotypes are still needed (e.g., <<CapsuleProperties>>) and so, in the case that an element has multiple stereotypes, it may still be best to show the keywords for all of them for clarity.  So I'd suggest raising a separate bugzilla for the stereotype keywords and restricting this one to the icon.

And a separate bugzilla for the metaclass tags, too.  There are a number of usability aspects to discuss in both.  All of this applies to Papyrus, I suppose, as other DSMLs besides UML-RT will have this concern.  But the problem at hand (icons) is specifically a Papyrus-RT problem because we implemented custom label providers that are broken :-)
Comment 7 Eclipse Genie CLA 2016-08-31 11:12:31 EDT
New Gerrit change created: https://git.eclipse.org/r/80138
Comment 8 Christian Damus CLA 2016-08-31 11:33:36 EDT
(In reply to Eclipse Genie from comment #7)
> New Gerrit change created: https://git.eclipse.org/r/80138

This is an incremental change to address the presentation of the selected element in the title bar of the properties view (the icons used in the diagram are a different issue):

* use the correct UML-RT icon for an element
* present the in/out direction kind for protocol messages
* use the tilde notation for the type of a conjugate port

So, basically, the Properties view shows exactly the same labels as the Model Explorer view.

Situations like using the label for the protocol message in the selection dialog for the accept-event for a transition trigger are necessarily one-offs.  These are cases where different labels are needed in different circumstances; the generic mechanism employed by Model Explorer and Properties views doesn't handle these requirements.
Comment 9 Christian Damus CLA 2016-08-31 11:35:40 EDT
(In reply to Christian W. Damus from comment #8)
> (In reply to Eclipse Genie from comment #7)
> > New Gerrit change created: https://git.eclipse.org/r/80138

I forgot to mention that this actually adds *more* stereotype keywords in the title bar of the Properties view, now.  Where properties (ports, capsule-parts) previously didn't have the <<RTPort>> and <<CapsulePart>> keywords, respectively, now they do.  But this is consistent with Model Explorer and other places, so that's probably a good thing.  Changing this behaviour would be in the scope of the related bug discussed in comment 6.
Comment 10 Eclipse Genie CLA 2016-08-31 17:02:36 EDT
New Gerrit change created: https://git.eclipse.org/r/80174
Comment 11 Christian Damus CLA 2016-08-31 17:07:22 EDT
(In reply to Eclipse Genie from comment #10)
> New Gerrit change created: https://git.eclipse.org/r/80174

This patch overrides the Papyrus UML composite structure diagram's default presentation of icons for the composite frame and for parts, which are based only on an element's metaclass, to use the more appropriate UML-RT icons.
Comment 12 Peter Cigehn CLA 2016-09-01 08:52:40 EDT
I took a quick look at the two Gerrit changes, and it looks like a good start. The title in the properties view is now at least consistent with the model explorer (but this then touches on the aspect that the stereotype label maybe should not be shown at all). Also checked the use of the customized icon in the diagrams, which looks good. 

One thing to note though regarding the icon usage in the diagrams. Here we probably should consider Bug 492652. The core intention with it is partly overlapping, but I think Rémi's intetion was to completely remove the icon in front of the name of a capsule part (since we have the adornment with the capsule icon in the top right corner of the capsule part). This is also consistent with legacy tooling which does not show an icon in front of the capsule part name. But if we now have changed the icon to not be the standard property icon, but the special capsule part icon (which looks very similar to the standard property icon, but it still is a specific icon), maybe the icon should be kept in front of the name. Anyway, this I guess should be handled in the scope of Bug 492652, i.e. whether to keep this update of using the proper capsule part icon, or if the icon should be removed completely. Just wanted to give a heads up regarding the existence of this Bugzilla.

Regarding your proposal for two separate Bugzillas in Comment 6, do you think that you could write and formulate those, at least the initial description? I am not fully sure what to write and how formulate them. As you say, we can continue the discussion in those then.
Comment 13 Christian Damus CLA 2016-09-01 10:23:04 EDT
(In reply to Peter Cigehn from comment #12)
> 
> One thing to note though regarding the icon usage in the diagrams. Here we
> probably should consider Bug 492652. The core intention with it is partly

Yes, I was wondering about the apparent redundancy of icons, but I wasn't aware of this bug.  I think the fix for bug 492652 would be in the same code that I added to fix the icon in the current case.  So, I could either

(a) update my patch on this bug to just remove the icon instead of replacing it, or
(b) undo my work in this bug that fixes the icon, since it will be going away
    completely, and leave it to Rémi in bug 492652 (which is assigned to him)

Have you any preference?  It should be a trivial increment on the work done so far to complete (a), and I don't expect that Rémi has done anything yet on the other bug, but I don't want to step on his toes.

It is also worth noting that eliminating the icon reduces the size of the label, which changes the automatic sizing of the capsule-part, which in turn may help visual fidelity of diagrams imported from RSA-RTE.


> Regarding your proposal for two separate Bugzillas in Comment 6, do you
> think that you could write and formulate those, at least the initial
> description? I am not fully sure what to write and how formulate them. As
> you say, we can continue the discussion in those then.

No problem.
Comment 14 Peter Cigehn CLA 2016-09-01 10:38:44 EDT
(In reply to Christian W. Damus from comment #13)
> (In reply to Peter Cigehn from comment #12)
> > 
> > One thing to note though regarding the icon usage in the diagrams. Here we
> > probably should consider Bug 492652. The core intention with it is partly
> 
> Yes, I was wondering about the apparent redundancy of icons, but I wasn't
> aware of this bug.  I think the fix for bug 492652 would be in the same code
> that I added to fix the icon in the current case.  So, I could either
> 
> (a) update my patch on this bug to just remove the icon instead of replacing
> it, or
> (b) undo my work in this bug that fixes the icon, since it will be going away
>     completely, and leave it to Rémi in bug 492652 (which is assigned to him)
> 
> Have you any preference?  It should be a trivial increment on the work done
> so far to complete (a), and I don't expect that Rémi has done anything yet
> on the other bug, but I don't want to step on his toes.
> 

I think it would be most efficient with (a), and that we resolve Bug 492652 at the same time, even though Rémi have it assigned to him. I know that Rémi have mentioned this Bugzilla at some time, and that he possibly had started with it. But I guess it is better if we fix this issue now since you are working on it.

> It is also worth noting that eliminating the icon reduces the size of the
> label, which changes the automatic sizing of the capsule-part, which in turn
> may help visual fidelity of diagrams imported from RSA-RTE.

Yes, that is true. There are a few other things that are different in the label which causes it to be longer, e.g. visibility (#) which is superfluous for capsule parts since it *always* is protected and multiplicity ([1]) for the case with non-replicated capsule parts, which differs from legacy as well where they are not shown. I think that there are a few more things here that we can improve... We need to iterate a bit on that also.
Comment 15 Christian Damus CLA 2016-09-01 12:21:29 EDT
(In reply to Peter Cigehn from comment #14)

I've updated the Gerrit patch to use CSS to remove the element icon entirely for capsule-parts as well as certain unnecessary particles in the mask label.  However, we also need to dynamically remove the multiplicity also from the label for non-replicated parts, and that can't be done (so it seems to me) in CSS, so we still need the custom name edit-part for that.

https://git.eclipse.org/r/80174
Comment 18 Christian Damus CLA 2016-09-01 13:58:36 EDT
(In reply to Eclipse Genie from comment #17)
> Gerrit change https://git.eclipse.org/r/80174 was merged to [master].

(In reply to Eclipse Genie from comment #16)
> Gerrit change https://git.eclipse.org/r/80138 was merged to [master].

The Properties view now presents the same labels in its title bar as the Model Explorer, the proper capsule icon is used in the structure diagram, and the labels of capsule-parts no longer contain redundant details.

One note for the last item:  currently, the [1] multiplicity is still shown on unreplicated capsule-parts because we will not have the "None (1)" replication option until the pending changes for bug 479352 are completed, which adds the Replication property for capsule-parts.  Until then, you can simulate the "none" replication by using the Advanced mode of the Model Explorer to find and delete the lowerValue and upperValue of the capsule-part.
Comment 19 Peter Cigehn CLA 2016-09-02 10:50:53 EDT
I was planning on verifying this in the latest Papyrus-RT build, but it looks like the builds have been failing so I cannot check this. Hopefully someone can take a look at why the builds have started to fail:

https://hudson.eclipse.org/papyrus-rt/job/Papyrus-RT-Product/
Comment 20 Peter Cigehn CLA 2016-09-05 08:38:52 EDT
(In reply to Christian W. Damus from comment #18)
> The Properties view now presents the same labels in its title bar as the
> Model Explorer, the proper capsule icon is used in the structure diagram,
> and the labels of capsule-parts no longer contain redundant details.
> 
> One note for the last item:  currently, the [1] multiplicity is still shown
> on unreplicated capsule-parts because we will not have the "None (1)"
> replication option until the pending changes for bug 479352 are completed,
> which adds the Replication property for capsule-parts.  Until then, you can
> simulate the "none" replication by using the Advanced mode of the Model
> Explorer to find and delete the lowerValue and upperValue of the
> capsule-part.

I've checked this in the latest Papyrus-RT build. The customized icons are now being used in the title of the properties view and the stereotype label is shown more consistently. Whether is actually shall be shown in all situations is tracked by Bug 500679.

Also checked that the redundant information for capsule parts are no longer shown, i.e. the property icon in front of the name is no longer shown, and neither is the visibility indicator (# for protected) nor the multiplicity for non-replicated capsule parts.

The definition of non-replicated though is both the case "None (1)" as well as the explicit case "1". Currently only the "None (1)" case removes the multiplicity in the label. If you explicitly set it to "1", then multiplicity it still shown as "[1]" in the label of the capsule part. In the legacy tooling, both the case of "None (1)" and "1" gets displayed without the "[1]" part in the label. Not sure if we should track this within the scope of this Bugzilla or not.

I put this one into verified since the main purpose of this Bugzilla has been fixed, i.e the use of the customized icons.
Comment 21 Christian Damus CLA 2016-09-06 10:58:18 EDT
(In reply to Peter Cigehn from comment #20)
> 
> The definition of non-replicated though is both the case "None (1)" as well
> as the explicit case "1". Currently only the "None (1)" case removes the
> multiplicity in the label. If you explicitly set it to "1", then
> multiplicity it still shown as "[1]" in the label of the capsule part. In
> the legacy tooling, both the case of "None (1)" and "1" gets displayed
> without the "[1]" part in the label. Not sure if we should track this within
> the scope of this Bugzilla or not.

Is it not useful, though, to indicate that a part's multiplicity is explicitly singular, not simply default as such?  Meaning that it is "replicated" exactly once?

If we want None and 1 to present the same in the diagram, it's easy to do.  I just want to be sure.
Comment 22 Peter Cigehn CLA 2016-09-06 11:12:14 EDT
(In reply to Christian W. Damus from comment #21)
> Is it not useful, though, to indicate that a part's multiplicity is
> explicitly singular, not simply default as such?  Meaning that it is
> "replicated" exactly once?
> 
> If we want None and 1 to present the same in the diagram, it's easy to do. 
> I just want to be sure.

Well, this is how the legacy tooling behaves. This about "None (1)", i.e. unset, vs. "1", i.e. explicitly set to 1, is actually just confusing (as we have discussed, and also comparable to the confusion about having visibility unset vs. setting visibility explicitly to public). But this confusion comes partly from the UML specification, and I don't really know how to best handle it, apart from simply mimic how it is done in the legacy tooling.

For a normal user the two cases "None (1)" and "1" have the same meaning, i.e. that is the definition of non-replicated. Hence also the notation with having the value 1 within parentheses for the "None (1)", i.e. unset, case. 

Replicated means 2 or more. 

To be noted also is that whenever you use a symbolic constants, i.e. an OpaqueExpression, then it should be considered replicated (we can only assume that the symbolic constant will have an actual value of 2 or more).

This about non-replicated vs replicated shall of course be aligned with the stacking pattern, i.e. the stacking pattern shall only be shown when it is replicated. And no stacking pattern shall be shown for neither of the cases "None (1)" or "1".
Comment 23 Christian Damus CLA 2016-09-06 12:36:22 EDT
A new patch to ensure that singular multiplicity is never shown:

https://git.eclipse.org/r/#/c/80506/

A further question:  do we want to show [0..1] for an unreplicated but optional (or plug-in) part?  Or should the multiplicity be suppressed in the label of all unreplicated parts?
Comment 24 Charles Rivet CLA 2016-09-06 13:26:16 EDT
(In reply to Peter Cigehn from comment #22)
> (In reply to Christian W. Damus from comment #21)
> > Is it not useful, though, to indicate that a part's multiplicity is
> > explicitly singular, not simply default as such?  Meaning that it is
> > "replicated" exactly once?
> > 
> > If we want None and 1 to present the same in the diagram, it's easy to do. 
> > I just want to be sure.
> 
> Well, this is how the legacy tooling behaves.

This kind of answer in a single sentence (and without a "but") always annoys me as we should thrive not only to replicate legacy tooling but also make it better, so I certainly understand the need to state it.

This about "None (1)", i.e.
> unset, vs. "1", i.e. explicitly set to 1, is actually just confusing (as we
> have discussed, and also comparable to the confusion about having visibility
> unset vs. setting visibility explicitly to public). But this confusion comes
> partly from the UML specification, and I don't really know how to best
> handle it, apart from simply mimic how it is done in the legacy tooling.

One big difference with legacy tooling is that we are trying to implement a more comprehensive DSML for UML-RT (we can talk in private about this statement if you want) and should endeavour to stay close to that DSML over what UML provides.

I think the two questions that should help in making that decision are:

* Will modeling users of legacy tooling be able to easily make the transition to a new approach?
* Will new modeling users be able to more quickly understand (or even care) about the underlying UML and correclty use a new approach?

Peter: You are probably in the best position to answer both of these.

> 
> For a normal user the two cases "None (1)" and "1" have the same meaning,
> i.e. that is the definition of non-replicated. Hence also the notation with
> having the value 1 within parentheses for the "None (1)", i.e. unset, case. 
> 
> Replicated means 2 or more. 
> 
> To be noted also is that whenever you use a symbolic constants, i.e. an
> OpaqueExpression, then it should be considered replicated (we can only
> assume that the symbolic constant will have an actual value of 2 or more).
> 
> This about non-replicated vs replicated shall of course be aligned with the
> stacking pattern, i.e. the stacking pattern shall only be shown when it is
> replicated. And no stacking pattern shall be shown for neither of the cases
> "None (1)" or "1".
Comment 25 Eclipse Genie CLA 2016-09-06 16:51:32 EDT
New Gerrit change created: https://git.eclipse.org/r/80506
Comment 26 Peter Cigehn CLA 2016-09-07 03:23:42 EDT
(In reply to Charles Rivet from comment #24)
> (In reply to Peter Cigehn from comment #22)
> > Well, this is how the legacy tooling behaves.
> 
> This kind of answer in a single sentence (and without a "but") always annoys
> me as we should thrive not only to replicate legacy tooling but also make it
> better, so I certainly understand the need to state it.

Yes, I fully understand this annoyance! I would be annoyed to. And yes, we do want to make the tooling better, and we have also done so in several areas. 

But please keep in mind, that this statement (in a single sentence without the "but") is given in specific context for a specific case. It's definitively not a general statement for all general cases of the tooling!

> 
> This about "None (1)", i.e.
> > unset, vs. "1", i.e. explicitly set to 1, is actually just confusing (as we
> > have discussed, and also comparable to the confusion about having visibility
> > unset vs. setting visibility explicitly to public). But this confusion comes
> > partly from the UML specification, and I don't really know how to best
> > handle it, apart from simply mimic how it is done in the legacy tooling.
> 
> One big difference with legacy tooling is that we are trying to implement a
> more comprehensive DSML for UML-RT (we can talk in private about this
> statement if you want) and should endeavour to stay close to that DSML over
> what UML provides.

Yes, I think that this is a perfect topic for discussing next week! And it is a topic that has been discussed rather extensively already from before in different areas of the tooling...

> 
> I think the two questions that should help in making that decision are:
> 
> * Will modeling users of legacy tooling be able to easily make the
> transition to a new approach?
> * Will new modeling users be able to more quickly understand (or even care)
> about the underlying UML and correclty use a new approach?
> 
> Peter: You are probably in the best position to answer both of these.
> 

There is also a third question to consider:

* How shall legacy models be handled when imported into Papyrus-RT?

If we did not have this question to consider then a lots of things would be so much easier in deciding how things should work.
Comment 27 Peter Cigehn CLA 2016-09-07 03:33:24 EDT
(In reply to Christian W. Damus from comment #23)
> A further question:  do we want to show [0..1] for an unreplicated but
> optional (or plug-in) part?  Or should the multiplicity be suppressed in the
> label of all unreplicated parts?

The legacy tooling do show [0..1] for an unreplicated but optional (or plugin-in) part. This feels a bit superfluous, since the optionality (lower bound of zero) is shown using the hatching pattern anyway. But I guess it is best to align with legacy to get the capsule part label to be more similar in length also in this case.
Comment 28 Christian Damus CLA 2016-09-07 08:25:50 EDT
(In reply to Peter Cigehn from comment #27)
> 
> The legacy tooling do show [0..1] for an unreplicated but optional (or
> plugin-in) part. This feels a bit superfluous, since the optionality (lower
> bound of zero) is shown using the hatching pattern anyway. But I guess it is
> best to align with legacy to get the capsule part label to be more similar
> in length also in this case.

Thanks, Peter!  This is what my current Gerrit patch (comment 25) does, so I shall go ahead and merge it that we may have done with this bug. ;-)  Besides that for visual-impairment accessibility reasons it's nice to have the label that a screen-reader can handle in addition to the hatching.
Comment 30 Peter Cigehn CLA 2016-09-27 08:19:44 EDT
Closing as verified fixed.