archi icon indicating copy to clipboard operation
archi copied to clipboard

[Enhancement] Execute TextRenderers in case no label expression exists (Archi 4.7)

Open pschoepf opened this issue 4 years ago • 23 comments

Hi, the new label expression feature coming with Archi 4.7 is a very nice feature. If I understand the feature correctly, label expressions reside on diagram objects only to in order to use it one would need to add such an expression to each diagram element.

In addition to that I was about to implement a small custom plugin which would allow to add generic text based on the value of a concept's property. The TextRenderer implementation would serve as a perfect infrastructure to implement this, as I would already be able to register a custom renderer to it. This way I would not need to implement custom Figure implementations to just change the text of a figure.

However currently the TextRenderer implementation skips the execution of any renderer in case the label expression is empty:

public String render(IArchimateModelObject object, String formatExpression) {
        if(!StringUtils.isSet(formatExpression)) {
            return "";
        }

changing the statement to:

if(!StringUtils.isSet(formatExpression)) {
            formatExpression = "";
        }

would already do what I need so I wanted to ask if its possible to change it ?

Another idea to takle this topic would be to extend the label expression feature to either allow the rendering through the diagram object label expression (as of now) OR based on a property value if that exists(name could be configurable through preferences or hard coded, treating the diagram object label expression with more priority if it exists). Such a feature would be great because it would allow to use label expressions for all components in a rush or just for single diagrams - and I would not need to implement a custom renderer for it as well.

if I get the code right the only necessary change on top of the one mention above would be to look for this property in getFormatExpression():

public String getFormatExpression(IArchimateModelObject object) {
        return object.getFeatures().getString(FEATURE_NAME, null);
    }

Thanks in advance, Philipp

pschoepf avatar Jun 24 '20 10:06 pschoepf

So the aim is to have a format expression for the underlying concept rather than each diagram instance. And the idea is to use the value of a property for the expression.

Iterating through all Text Renderers even if the format expression is empty would mean that a Text Renderer could return the label text based on some other criteria such as a Property. That could be done with little impact on performance if each inbuilt Text Renderer checked for the empty string and returned it in public String render(IArchimateModelObject object, String text)

But I have my doubts about using a Property value. For performance reasons we need to ensure that a diagram containing hundreds of objects (and the Models tree) spends as little time as possible parsing the rendering logic.

Our aim was not to use a property but an internal "feature" for the label expression and to implement it on per instance basis.

I need to think about this...

Phillipus avatar Jun 24 '20 12:06 Phillipus

I was about to implement a small custom plugin which would allow to add generic text based on the value of a concept's property.

TBH I don't really understand what you want to achieve. Could you describe your use-case a bit more and provide some example of what this would do ?

Edit: What I'm trying to see is if this fits in the medium/long term goal we have around ArchiMate profiles and stereotypes (which will requires us to be able to define per stereotype attributes such as figure, color, label...

jbsarrodie avatar Jun 24 '20 12:06 jbsarrodie

My understanding is:

  • Also allow label rendering for the concept (rather than the diagram instance)
  • Also allow to store the format expression in the concept's property
  • Allow Text Renderers to process public String render(IArchimateModelObject object, String text) even if the object's format expression is the empty string

Phillipus avatar Jun 24 '20 13:06 Phillipus

So it seems that setting the label expression for each diagram instance is useful, but so is setting it at the underlying concept so that the label expression is the same wherever the concept appears.

Phillipus avatar Jun 24 '20 13:06 Phillipus

My understanding is:

Not so clear to me...

  • Also allow label rendering for the concept (rather than the diagram instance)

Some variant will be needed for stereotypes, so +1 on this

  • Also allow to store the format expression in the concept's property

IMHO we should not use a property (visible to the end user) for this, but instead use a Feature attached to the concept (and even in this case I don't see the use-case).

  • Allow Text Renderers to process public String render(IArchimateModelObject object, String text) even if the object's format expression is the empty string

This could allow a custom renderer to inject some custom label, so why not (but again I don't have a use-case in mind)

jbsarrodie avatar Jun 24 '20 13:06 jbsarrodie

I don't like the idea of using Properties for UI metadata, that's why we have internal "features".

One way to support label expression for the concept would be to store a label expression in a feature for the concept and also for the diagram object and allow one to over-ride the other. But this would then mean having two label expression text boxes and an option to over-ride the concept's label expression (if the diagram object label expression is not empty use that)

Phillipus avatar Jun 24 '20 13:06 Phillipus

Just checked the code. It's doable but would add on another few days for implementation and testing, and perhaps another week for beta testing.

The UI would have to look something like this:

Image 1

Phillipus avatar Jun 24 '20 13:06 Phillipus

Just checked the code. It's doable but [...]

But I still don't see the use-case. I'd like @pschoepf to provide us more details before even thinking about adding one more attribute in the UI (we don't want to look like other modelling tools, do we ?)

jbsarrodie avatar Jun 24 '20 13:06 jbsarrodie

But I still don't see the use-case.

Isn't it to set the label expression once per concept so it appears the same in all diagram instances?

Phillipus avatar Jun 24 '20 13:06 Phillipus

Hi both, thanks for looking into this and already having an in-depth conversation. It is exactly as Phil summarised in his understanding. Use case for us is as follows: We maintain a set of standard attributes on our concepts (e.g. application component carries an "component-type" property which can be either "Application" or "Component"). It would be nice to have the value of this property "out of the box" visible in all diagrams once we drop in the concept. Currently we we need to set this per diagram object and again and again for new diagrams. I guess the use case is a lightweight use case of what the specialization plugin does. +1 for Phil's draft specification above

pschoepf avatar Jun 24 '20 13:06 pschoepf

Use case for us is as follows: We maintain a set of standard attributes on our concepts (e.g. application component carries an "component-type" property which can be either "Application" or "Component". It would be nice to have the value of this property "out of the box" visible in all diagrams once we drop in the component.

Thank you.

In this case this is not a per concept label, but a per (stereo)type label which is not the same as what Phil was showing.

So this is one of the use-case we'll have to support for ArchiMate profiles and stereotypes and this requires some model-wide configuration to define the list of stereotypes, their properties and their default (visual) attribute such as labels.

In essence this is very similar to what Herve did implement in his second version of the specialization plugin (you have to test it yourself because there is no screenshots online for this part)

jbsarrodie avatar Jun 24 '20 13:06 jbsarrodie

currently we we need to set this per diagram object and again and again for new diagrams.

This can be done through jArchi quite easily (but that's only a workaround)

jbsarrodie avatar Jun 24 '20 13:06 jbsarrodie

Obviously setting the label for each diagram object is a good feature. But I can also see users wanting to set it just once for every instance.

Workarounds are copy/paste or:

This can be done through jArchi quite easily (but that's only a workaround)

Phillipus avatar Jun 24 '20 13:06 Phillipus

It turns out that adding an additional field for a global label expression and querying it was quite trivial. I've committed some changes in a new branch, label-render-global.

If a concept is selected in a view there are two label expression fields - one for the diagram object and one for the concept. Any label expression for the diagram object over-rides the one for the concept.

Allow Text Renderers to process public String render(IArchimateModelObject object, String text) even if the object's format expression is the empty string

This is possible but goes against the optimisations I've implemented. Because refreshing all labels in all views that are open can be an expensive operation we have a method:

TextRenderer#hasFormatExpression(object)

Every time the model changes a View will update the text labels of all of its objects if the object has a format expression. If we now have it so that all objects are updated whether there is a text expression or not we will see a slow down.

This is also an optimisation that is used when refreshing labels in the Models Tree - this can really slow down if we are updating all labels.

Phillipus avatar Jun 24 '20 16:06 Phillipus

Let's try to summarize the use-cases we want to support in the future to see how this fits...

Label customization should be part of a broader support for profiles and stereotypes (together with ability to choose the figure from a bigger list that only the defaults defined in the ArchiMate specifications). So we want at some point to be able to define per stereotype default labels.

@pschoepf use case is to define such default label per concept type, not per concept instance. This is somewhat similar to the label we can set on folders (it applies automatically to all concept inside the folder) except, of course, that this one has to be per type and applied on views. I did have feedback from users wanted this (I'm part of those users btw).

What @Phillipus described is a bit different because it is a per instance label, meaning that people would still have to edit the label for each element in the model, but once done this label would be used by default in a view. TBH I haven't had any feedback yet from users asking fo this and I don't see a use-case behind (which doesn't mean there ain't).

@Phillipus Adressing the first use-case requires us to be able to define per (stereo)type label. But where can we set this ? On option could be to have a "Default Label" tab added to Model's properties. Then default labels could be stored on a Feature whose name is based on the target concept type.

This is possible but goes against the optimisations I've implemented. Because refreshing all labels in all views that are open can be an expensive operation we have a method:

Defining per concept type labels raises the same concern: as soon as you've defined that any Application Component should have a label set such as ${name} [${property:component-type}], then all Application Components in any view will run the renderers.

Another way of implementing this (almost a workaround but maybe more aligned with what we already have so far) would be to add an option for using the folder-defined label as default label (similar to the option we have to use it in analysis tab). Then it would be up to the user to define the folder label expression to $model{property:default-label-for-${type}} and manually create model level properties named default-label-for-application-component with value ${name} [${property:component-type}]. Of course, doing so would require to also define a similar property (with value being ${name} for all concept's types of the layer matching the folder. This also means that the naming seen in folders will be the default one (but one could want to distinguish them).

jbsarrodie avatar Jun 24 '20 19:06 jbsarrodie

This is starting to sound a bit complicated at this late stage of (almost) releasing Archi 4.7. ;-)

At this stage the only change I feel comfortable with is;

What @Phillipus described is a bit different because it is a per instance label, meaning that people would still have to edit the label for each element in the model, but once done this label would be used by default in a view.

Or no changes until 4.8 (I think I've run out of steam on this feature)

Phillipus avatar Jun 24 '20 19:06 Phillipus

This is starting to sound a bit complicated at this late stage of (almost) releasing Archi 4.7. ;-)

IMHO this is already too late to add anything in 4.7. I would suggest to keep this for 4.8 (which doesn't have to be in 8 months)

jbsarrodie avatar Jun 24 '20 19:06 jbsarrodie

IMHO this is already too late to add anything in 4.7. I would suggest to keep this for 4.8 (which doesn't have to be in 8 months)

I think so too. I think we should spend more time on this later and try to align it with a broader scope (profiles/stereotypes).

Edit - also it allows us time to gather more user feedback.

Phillipus avatar Jun 24 '20 19:06 Phillipus

I see, it gets complicated :-)

Just to clarify: may be my example use case above was written too quick and did not get to the point of my original idea which was indeed to have the label expression per concept instance (as outlined by @Phillipus solution) and not once per concept type. That's why I proposed to have it in a property - which I understand would be the wrong place to put it because of various reasons. Having it on concept type would as described by @jbsarrodie also work for my example but it was not my initial intention.

Still reading all the thoughts above may be its better to give it some more time. But thanks for having the discussion.

pschoepf avatar Jun 25 '20 07:06 pschoepf

my original idea which was indeed to have the label expression per concept instance

That is possible and this is what I've done in the label-render-global branch, but a comment from @jbsarrodie :

TBH I haven't had any feedback yet from users asking fo this and I don't see a use-case behind (which doesn't mean there ain't).

So, is this a desirable feature? It would mean having two label expression text boxes. How many instances of a concept does one typically re-use? Is it sufficient to copy and paste an instance to re-use its label expression?

Phillipus avatar Jun 25 '20 07:06 Phillipus

@pschoepf We've decided not to make any last minute changes to this for Archi 4.7. We'd like to get user feedback for this feature as it is, and take a bit of time to assess that feedback. It may be that we expand upon this feature in line with a broader scope such as defining a profile that includes a label expression. But we'll keep this issue open.

Phillipus avatar Jun 29 '20 20:06 Phillipus

my original idea which was indeed to have the label expression per concept instance

That is possible and this is what I've done in the label-render-global branch, but a comment from @jbsarrodie :

TBH I haven't had any feedback yet from users asking fo this and I don't see a use-case behind (which doesn't mean there ain't).

So, is this a desirable feature? It would mean having two label expression text boxes. How many instances of a concept does one typically re-use? Is it sufficient to copy and paste an instance to re-use its label expression?

From my expirience a key concept may be reused up to tens of times.

WatchTh1 avatar Jul 01 '20 18:07 WatchTh1

Could I suggest that may be views should have attribute of context where labels a default set by context. For instance if I have a high level diagram I may wish to show description but at a lower level exclude description. Or if context if diagram is costs, it details such in diagram Plus it would be nice at higher levels to show relationship name such as flow based on lower level objects materialising flow. It seems to me such should be readily implemented by user defining contexts in configuration which are picked up and applied as the default behaviour.

peterprib avatar Oct 20 '22 02:10 peterprib