[doc] Add support for styled labels in view model for trees

[jerome-obeo]

Pull request template

General purpose

What is the main goal of this pull request?

• Bug fixes
• New features
• Documentation
• Cleanup
• Tests
• Build / releng

Project management

• Has the pull request been added to the relevant project and milestone? (Only if you know that your work is part of a specific iteration such as the current one)
• Have the priority: and pr: labels been added to the pull request? (In case of doubt, start with the labels priority: low and pr: to review later)
• Have the relevant issues been added to the pull request?
• Have the relevant labels been added to the issues? (area:, difficulty:, type:)
• Have the relevant issues been added to the same project and milestone as the pull request?
• Has the CHANGELOG.adoc been updated to reference the relevant issues?
• Have the relevant API breaks been described in the CHANGELOG.adoc? (Including changes in the GraphQL API)
• In case of a change with a visual impact, are there any screenshots in the CHANGELOG.adoc? For example in doc/screenshots/2022.5.0-my-new-feature.png

Architectural decision records (ADR)

• Does the title of the commit contributing the ADR start with [doc]?
• Are the ADRs mentioned in the relevant section of the CHANGELOG.adoc?

Dependencies

• Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc?
• Are the new dependencies justified in the CHANGELOG.adoc?

Frontend

This section is not relevant if your contribution does not come with changes to the frontend.

General purpose

• Is the code properly tested? (Plain old JavaScript tests for business code and tests based on React Testing Library for the components)

Typing

We need to improve the typing of our code, as such, we require every contribution to come with proper TypeScript typing for both changes contributing new files and those modifying existing files.
Please ensure that the following statements are true for each file created or modified (this may require you to improve code outside of your contribution).

• Variables have a proper type
• Functions’ arguments have a proper type
• Functions’ return type are specified
• Hooks are properly typed:
  • useMutation<DATA_TYPE, VARIABLE_TYPE>(…)
  • useQuery<DATA_TYPE, VARIABLE_TYPE>(…)
  • useSubscription<DATA_TYPE, VARIABLE_TYPE>(…)
  • useMachine<CONTEXT_TYPE, EVENTS_TYPE>(…)
  • useState<STATE_TYPE>(…)
• All components have a proper typing for their props
• No useless optional chaining with ?. (if the GraphQL API specifies that a field cannot be null, do not treat it has potentially null for example)
• Nullable values have a proper type (for example let diagram: Diagram | null = null;)

Backend

This section is not relevant if your contribution does not come with changes to the backend.

General purpose

• Are all the event handlers tested?
• Are the event processor tested?
• Is the business code (services) tested?
• Are diagram layout changes tested?

Architecture

• Are data structure classes properly separated from behavioral classes?
• Are all the relevant fields final?
• Is any data structure mutable? If so, please write a comment indicating why
• Are behavioral classes either stateless or side effect free?

Review

How to test this PR?

Please describe here the various use cases to test this pull request

• Has the Kiwi TCMS test suite been updated with tests for this contribution?

[sbegaudeau]

My first gripe with this is that none of this is particularly relevant to sole the problem described. It does not matter at all to support styled labels. It may be useful but it's definitely not the heart of the issue nor required. This is completely optional, start the ADR by the core of the solution.

The first major problem is defining the solution not how to organize it.

[adaussy]

I'm not sure what you mean?

For me the purpose of the "advanced" ADR was to agree on the technical stuff now, so afterward we could be autonomous with Florian?

[sbegaudeau]

Can you really make that work while keeping a single treeItemLabelExpression?

[adaussy]

The idea was that this expression would be used if ConditionalStyle predicate returns false.
I think in most use cases, users would not define style for their items and use the default style provided by the appplication (maybe I'm wrong). So by having this fallback mechanism it would reduce the complexity of defining a TreeDescription.

[sbegaudeau]

Why is there another label expression here? I mean I understand this one, not the other one DiagramDescription#treeItemLabelExpression

[adaussy]

See above

[sbegaudeau]

I don't think that it can realistically work for a couple of reason:

• I would need to both define a label and another version of the label in case I want to have some fragments
• This is not dynamic enough to be usable
• You can't just consider the order the fragment like that, it won't work, you can't really sort all. The potential use case of a metamodel using one list.

Consider some my current use cases with the Papaya metamodel. It's a bit like a metamodel of the Java language. How can I, with your API create a description to let me specific the color for those examples:

Method types

• I have some operations with parameters, there can be any number of parameters in the operations of my models from 0 to god knows how many.
• I want the type of each value and the return type of the method in blue
• I want to support displaying the parenthesis in grey

Keyword and generic types

• I have classes, interfaces and records which can have generic types
• They can have any number of generic type whatsoever
• I want to support displaying keywords in purple
• I want to support displaying generic types in light blue
• I want to support displaying < and > in grey

And finally, I don't want to manually define all the potential combinations of all the potential use cases. So in the case of my operations, I don't want to create a fragment for all the potential number of parameters in an operations that can appear. Just try to order all the fragments to make it work, it's not viable.

Unless I missed something, and I would be happy to be proved wrong, this approach cannot possibly work beyond basic use cases. With this approach based on conditional style, you are taking an existing solution (a hammer) and you are looking for a nail but it won't work.

I think you need to start using complex examples as the target to reach and stay focused on how to express potential solutions. You can't assume that the specifier will know at build time the number of fragments that will be required, models expressed by end users are too complex for such assumptions. You may need to start taking inspiration from our work on dynamic mappings in the form DSL which encountered the same issue. You could end up with something like this:

• A new field TreeDescription#labelDescriptions: LabelDescription[1..*]

TreeItemLabelDescription {
  name: Identifier
  preconditionExpression: InterpretedExpression<boolean>
  children: TreeItemLabelElementDescription[1..*] // TreeItemLabelElementDescription is abstract
}

ForTreeItemLabelElementDescription extends TreeItemLabelElementDescription {
  iterator: String
  iterableExpression: InterpretedExpression<List<Object>>
  children: TreeItemLabelElementDescription[1..*]
}

IfTreeItemLabelElementDescription extends TreeItemLabelElementDescription {
  predicateExpression: InterpretedExpression<boolean>
  children: TreeItemLabelElementDescription[1..*]
}

TreeItemLabelFragmentDescription extends TreeItemLabelElementDescription {
  labelExpression: string
  colorExpression: InterpretedExpression<string>
  // All the other properties, it's not that relevant
}

With this, you could end up supporting the operation example like that:

TreeDescription {
  labelDescriptions: [
    LabelDescription {
      name: "Operation"
      preconditionExpression: "aql:self.eClass() = papaya::Operation"
      children: [
        TreeItemLabelFragmentDescription {
          labelExpression: "aql:self.name"
          colorExpression: "black"
        },

        TreeItemLabelFragmentDescription {
          labelExpression: "("
          colorExpression: "grey"
        },

        ForTreeItemLabelElementDescription {
          iterator: "parameter"
          iterableExpression: "aql:self.parameters"
          children: [
            TreeItemLabelFragmentDescription{
              labelExpression: "aql:parameter.name",
              colorExpression: "black"
            },
            TreeItemLabelFragmentDescription{
              labelExpression: "aql:parameter.type.name",
              colorExpression: "blue"
            },
            IfTreeItemLabelElementDescription{
              predicateExpression: "aql:self.parameters.indexOf(parameter) <> (self.parameters.size() -1))
              children: [
                TreeItemLabelFragmentDescription{
                  labelExpression: ","
                  colorExpression: "black"
                }
              ]
            },
          ]
        },

        TreeItemLabelFragmentDescription {
          labelExpression: ")"
          colorExpression: "grey"
        },

        TreeItemLabelFragmentDescription {
          labelExpression: ":"
          colorExpression: "black"
        },

        TreeItemLabelFragmentDescription {
          labelExpression: "aql:self.type.name"
          colorExpression: "blue"
        },
      ]
    }
  ]
}

(I know, I'm not considering spaces but it's simple to add)

Now unless you can prove me that your solution can work to create the labels that I have given you as an example, prove me that my solution here cannot work even for my most complex examples. The first thing I care about is not creating a palette of TextStyle, it's completely optional, I want a solution which can work for complex use cases.

[adaussy]

I totally agree with you solution for complex labels. But initial the Form DSL dit not provide the dynamic mapping at the begining so this solution would have been a first step (and sufficient for our use case). Afteward, we could have added the dynamic item to express the if and loop feature.

I agree that the complex example should be taken in consideration now to avoid any API break in the future. So you examples are good to keep in mind but it was not our intention to handle them as a first step.

Do you think that a first stage that do not handle complexe example as your is acceptable, or should we implement it now? (I'm affraid for my budget)

As a first step, we could introduce an abstract class that would allow us to extend with dynamic concepts afterwards?

Moreover, in our solution we add the capacity to express a meaninfull style. For example, as a specifier I want to create a Read Only style that display a text in italic and grey (or any another combination). Then I want to be able to reuse that style in many different scenario. The fact to link the the precondition, labe expression and the definition of the style make them not really reusable. It would mean for the specifier to dupplicate many time the colorExpression, isItalicExpresion and so on.

For example, in your example the "Black" style is repeated a lot of time. With our solution we would have a TextStyleDescription name "Default Style" with difine this "Black Color". Then it would be referenced any time needed.

[sbegaudeau]

All this is fine and nice but completely secondary to bring support for existing styled labels in the tree DSL. That's about improving what exists today, ok. It's an entirely different matter.

[adaussy]

Yes we could have created a second ADR for those but we wanted you to give us all your feedback in one time to reduce the loop cost.

[sbegaudeau]

Just to try another use case, something like the image above could then be expressed like that:

TreeDescription {
  labelDescriptions: [
    LabelDescription {
      name: "Class"
      preconditionExpression: "aql:self.eClass() = papaya::Interface"
      children: [
        TreeItemLabelFragmentDescription {
          labelExpression: "aql:self.visibility + ' '"
          colorExpression: "purple"
        },

        TreeItemLabelFragmentDescription {
          labelExpression: "interface "
          colorExpression: "purple"
        },


        TreeItemLabelFragmentDescription {
          labelExpression: "aql:self.name"
          colorExpression: "black"
        },

        IfTreeItemLabelElementDescription{
          predicateExpression: "aql:self.genericTypes.size() > 0"
          children: [
            TreeItemLabelFragmentDescription {
              labelExpression: "<"
              colorExpression: "grey"
            },

            ForTreeItemLabelElementDescription {
              iterator: "genericType"
              iterableExpression: "aql:self.genericTypes"
              children: [
                TreeItemLabelFragmentDescription{
                  labelExpression: "aql:genericType.name + ' '",
                  colorExpression: "turquoise"
                },
                IfTreeItemLabelElementDescription{
                  predicateExpression: "aql:self.genericTypes.indexOf(genericType) <> (self.genericTypes.size() -1))
                  children: [
                    TreeItemLabelFragmentDescription{
                      labelExpression: ","
                      colorExpression: "black"
                    }
                  ]
                },
              ]
            },

            TreeItemLabelFragmentDescription {
              labelExpression: ">"
              colorExpression: "grey"
            },
          ]
        },

        IfTreeItemLabelElementDescription{
          predicateExpression: "aql:self.extends.size() > 0"
          children: [
            TreeItemLabelFragmentDescription {
              labelExpression: " extends "
              colorExpression: "purple"
            },

            ForTreeItemLabelElementDescription {
              iterator: "extendedInterface"
              iterableExpression: "aql:self.extends"
              children: [
                TreeItemLabelFragmentDescription{
                  labelExpression: "aql:extendedInterface.name + ' '",
                  colorExpression: "black"
                },

                // FAILURE

                IfTreeItemLabelElementDescription{
                  predicateExpression: "aql:self.extendeds.indexOf(extendedInterface) <> (self.extends.size() -1))
                  children: [
                    TreeItemLabelFragmentDescription{
                      labelExpression: ","
                      colorExpression: "black"
                    }
                  ]
                },
              ]
            },
          ]
        }

      ]
    }
  ]
}

And here you have a failure of my language! It cannot create recursively the same structure. I cannot express in a generic way something like this:

So the next issue to be solved, is an issue of reusability but not the reusability of the style, it does not matter since it does not prevent me from creating what I need but instead a reusability of the logic to match what I want to express.

[sbegaudeau]

This is something that we have solved already with the reusability of NodeDescription as children. For example for a component diagram where components can be children of themselves. But there is one critical difference in our case, we need to control the order which is not the case in a diagram (nobody cares about the order of both NodeDescription#childrenDescriptions and NodeDescription#reusedChildNodeDescriptions). So the solution needs to be different, something like this maybe:

ReusedTreeItemLabelElementDescription extends TreeItemLabelElementDescription {
  elements: TreeItemLabelElementDescription[0..*]
}

to let us create something like that:

TreeDescription {
  labelDescriptions: [
    LabelDescription {
      name: "Class"
      preconditionExpression: "aql:self.eClass() = papaya::Interface"
      children: [
        TreeItemLabelFragmentDescription {
          labelExpression: "aql:self.visibility + ' '"
          colorExpression: "purple"
        },

        TreeItemLabelFragmentDescription {
          labelExpression: "interface "
          colorExpression: "purple"
        },


        TreeItemLabelFragmentDescription {
          labelExpression: "aql:self.name"
          colorExpression: "black"
        },

        IfTreeItemLabelElementDescription{
          predicateExpression: "aql:self.genericTypes.size() > 0"
          children: [
            TreeItemLabelFragmentDescription {
              labelExpression: "<"
              colorExpression: "grey"
            },

            ForTreeItemLabelElementDescription {
              iterator: "genericType"
              iterableExpression: "aql:self.genericTypes"
              children: [
                TreeItemLabelFragmentDescription{
                  labelExpression: "aql:genericType.name + ' '",
                  colorExpression: "turquoise"
                },
                IfTreeItemLabelElementDescription{
                  predicateExpression: "aql:self.genericTypes.indexOf(genericType) <> (self.genericTypes.size() -1))
                  children: [
                    TreeItemLabelFragmentDescription{
                      labelExpression: ","
                      colorExpression: "black"
                    }
                  ]
                },
              ]
            },

            TreeItemLabelFragmentDescription {
              labelExpression: ">"
              colorExpression: "grey"
            },
          ]
        },

        IfTreeItemLabelElementDescription{
          predicateExpression: "aql:self.extends.size() > 0"
          children: [
            TreeItemLabelFragmentDescription {
              labelExpression: " extends "
              colorExpression: "purple"
            },

            ForTreeItemLabelElementDescription {
              iterator: "extendedInterface"
              iterableExpression: "aql:self.extends"
              children: [
                ReusedItemLabelFragmentDescription{
                   element: non containment reference to the name, the generic types, the extends etc.
                },

                IfTreeItemLabelElementDescription{
                  predicateExpression: "aql:self.extendeds.indexOf(extendedInterface) <> (self.extends.size() -1))
                  children: [
                    TreeItemLabelFragmentDescription{
                      labelExpression: ","
                      colorExpression: "black"
                    }
                  ]
                },
              ]
            },
          ]
        }

      ]
    }
  ]
}

This would work.

Since it should allow us to reuse pretty much anything, I don't think we even need a palette for styles at all.

[sbegaudeau]

With this latest solution, I can:

• Use fragments conditionally thanks to the "If"
• Use a dynamic number of fragments thanks to the "For"
• Reuse any number of parts of another label thanks to the "Reuse"
• Create recursive labels thanks to the "Reuse"

Is there any use case that I'm missing?

[adaussy]

Just to try another use case, something like the image above could then be expressed like that:

TreeDescription {
  labelDescriptions: [
    LabelDescription {
      name: "Class"
      preconditionExpression: "aql:self.eClass() = papaya::Interface"
      children: [
        TreeItemLabelFragmentDescription {
          labelExpression: "aql:self.visibility + ' '"
          colorExpression: "purple"
        },

        TreeItemLabelFragmentDescription {
          labelExpression: "interface "
          colorExpression: "purple"
        },


        TreeItemLabelFragmentDescription {
          labelExpression: "aql:self.name"
          colorExpression: "black"
        },

        IfTreeItemLabelElementDescription{
          predicateExpression: "aql:self.genericTypes.size() > 0"
          children: [
            TreeItemLabelFragmentDescription {
              labelExpression: "<"
              colorExpression: "grey"
            },

            ForTreeItemLabelElementDescription {
              iterator: "genericType"
              iterableExpression: "aql:self.genericTypes"
              children: [
                TreeItemLabelFragmentDescription{
                  labelExpression: "aql:genericType.name + ' '",
                  colorExpression: "turquoise"
                },
                IfTreeItemLabelElementDescription{
                  predicateExpression: "aql:self.genericTypes.indexOf(genericType) <> (self.genericTypes.size() -1))
                  children: [
                    TreeItemLabelFragmentDescription{
                      labelExpression: ","
                      colorExpression: "black"
                    }
                  ]
                },
              ]
            },

            TreeItemLabelFragmentDescription {
              labelExpression: ">"
              colorExpression: "grey"
            },
          ]
        },

        IfTreeItemLabelElementDescription{
          predicateExpression: "aql:self.extends.size() > 0"
          children: [
            TreeItemLabelFragmentDescription {
              labelExpression: " extends "
              colorExpression: "purple"
            },

            ForTreeItemLabelElementDescription {
              iterator: "extendedInterface"
              iterableExpression: "aql:self.extends"
              children: [
                TreeItemLabelFragmentDescription{
                  labelExpression: "aql:extendedInterface.name + ' '",
                  colorExpression: "black"
                },

                // FAILURE

                IfTreeItemLabelElementDescription{
                  predicateExpression: "aql:self.extendeds.indexOf(extendedInterface) <> (self.extends.size() -1))
                  children: [
                    TreeItemLabelFragmentDescription{
                      labelExpression: ","
                      colorExpression: "black"
                    }
                  ]
                },
              ]
            },
          ]
        }

      ]
    }
  ]
}

And here you have a failure of my language! It cannot create recursively the same structure. I cannot express in a generic way something like this:

So the next issue to be solved, is an issue of reusability but not the reusability of the style, it does not matter since it does not prevent me from creating what I need but instead a reusability of the logic to match what I want to express.

Again in the diagram DSL, the reused mapping was introduce as a second step.

I'm really not sure to have the budget to handle all those case in my current projet. I guess we need to discuss that.

[adaussy]

With this latest solution, I can:

• Use fragments conditionally thanks to the "If"
• Use a dynamic number of fragments thanks to the "For"
• Reuse any number of parts of another label thanks to the "Reuse"
• Create recursive labels thanks to the "Reuse"

Is there any use case that I'm missing?

No I think you got it all, but do you think that we could split this development effort in several steps?

[adaussy]

We are going to rewrite this ADR taking your remarks in consideration but I need to understand if you are expecting us to implements all those features or if we could split the effort to stop when we have everthing we need; or you want all those feature in one PR?

[jerome-obeo]

I rewrote the ADR to fit with the comments:

• only one property in TreeDescription for expressing style descriptions.
• handle complex control flow for fragment description (if, for, and reuse)
• fragments reference TextStyledDescription that are stored inside a root TextStylePalette similar to ColorPalette
• remove some details aside from the main work (for instance missing text style properties bold and italic)

[adaussy]

Please specifiy it is non containment reference

[jerome-obeo]

Done

[adaussy]

typo after argument

[jerome-obeo]

Done

[adaussy]

I guess here it would be more a reference to a BackLabelStyle TextStyleDescription

[jerome-obeo]

Done