Bug 527121 - Select renderer in view model
Summary: Select renderer in view model
Status: CLOSED WONTFIX
Alias: None
Product: ECP
Classification: Modeling
Component: EMF Forms (show other bugs)
Version: 1.14.0   Edit
Hardware: PC Mac OS X
: P3 enhancement (vote)
Target Milestone: backlog   Edit
Assignee: Eugen Neufeld CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2017-11-10 08:37 EST by Hallvard Traetteberg CLA
Modified: 2019-06-14 05:46 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hallvard Traetteberg CLA 2017-11-10 08:37:36 EST
I've made a new SWT renderer for editing boolean attributes in a dropdown with false/no and true/yes options. This provides better usability in some cases.

To use it, you must create a service that gives it priority above the default one. I find this cumbersome, since you must add code for a rather simple case, which goes against the philosophy of EMF Forms. It would be better if you could add settings to a control element in the view model, that give hints for the selection or directly selects a particular renderer.

Note that this is relevant in many cases where the type of an attribute is not enough to select the "best" widget. E.g. if an int is in some small range, a slider is better, a String may in practice represent an enum, so a drop down is best etc. Note also, that in many cases you may want to provide additional setting to the renderer, e.g. the range for an int or possible values for a String.

A simple solution is to allow registering a renderer using an extension point, and provide the datatype it supports, e.g. EBoolean in my case, as well as a "style" key for selecting it, e.g. "dropDown". The control element of the view model can have a corresponding style key attribute, so you can indicate that you prefer a specific renderer (if it is supported by the runtime platform).

An slightly more complex, but more flexible solution is to make the control element support a renderer child element (of class Renderer) that can both provide the style key, as well as other configuration options, e.g. in my case which labels to use for the true and false cases. Having an extensible renderer model element, that can be used for selecting and configuring renderers, will increase the power considerably. So when you create a new renderer, you may also add an Ecore model with a Renderer subclass with features that allow the renderer to be configured. The instance of this Renderer must be provided when activating the actual renderer.

I understand there's a balance between simplicity and complexity, but by supporting a renderer sub-element, you don't reduce the simplicity much, but increase the power considerably.
Comment 1 Eugen Neufeld CLA 2018-02-13 05:25:53 EST
We're happy to accept a contribution here. Moving due to lack of time.
Comment 2 Eugen Neufeld CLA 2018-06-14 04:30:20 EDT
I thought over your suggestion and the biggest problem with your suggestion is, that the user has to set the magic key on all controls.

In my opinion you bring up the right argument, that if an int has a (small) range defined that instead of the normal int renderer a slider should be used, if the range is really small (eg less then 3 different values) a dropdown is more usable. But in my opinion the conclusion should not be to let the user set a magic key on the control but instead the tester of the different renderers should check the EAttributes range (which is easily possible) and take it into account when returning whether they are applicable for rendering or not. This way you get a consistent behavior out of the box without a user which can forget to set a style on a control.

What we currently suggest to do if you want to replace a specific control in a specific place in your view model is to add an annotation to it (VAnnotation) which allows you to set a key-value pair. In your tester you can then check for this annotation.

Of course you can still implement your own registration mechanism by writing one tester which parses the extension point you define and returns the applicability based on those information. You can look at the different implementations of org.eclipse.emfforms.spi.swt.core.EMFFormsRendererService<VELEMENT> .
You can also extend the VControl with your own and add the necessary style attribute.

Cheers,
Eugen
Comment 3 Hallvard Traetteberg CLA 2018-06-14 04:49:42 EDT
Thanks for considering my proposal. A mechanism like I propose would noe require you to provide anything, the default behaviour should be like the current, the system selects based on applicable renderers. It's only when several match that it would consider extra info you may have added for selecting the most appropriate one. Relying on the logic of the tester will not work for the renderer I made, how would it know if a yes/no dropdown is better than a checkbox?

Your suggestion to use a VAnnotation (I didn't know such a thing existed) should work for simple cases. In practice, the key/value pairs used by a particular tester would be a kind of "mini-model" for configuring it. My proposal can be thought of as a way of making it explicit, rather than ad-hoc. But it is a start!

I may still look into a possible implementation of my proposal, I hope you still are interested in reviewing it
Comment 4 Hallvard Traetteberg CLA 2018-06-14 05:46:45 EDT
I didn't find the VAnnotation class, did you mean VAttachment?
Comment 5 Hallvard Traetteberg CLA 2018-06-14 06:07:27 EDT
(In reply to Hallvard Traetteberg from comment #4)
> I didn't find the VAnnotation class, did you mean VAttachment?
VAttachment doesn't have key/value pairs either, although we could make a subclass with it. EAnnotations cannot be used, since they are not naturally part of a view model.
Comment 6 Eugen Neufeld CLA 2018-06-14 06:37:44 EDT
It's not part of the core model, but so is not the VTable.
The VAnnotation extends the VAttachment so you can always add it.
If I understand your use case correctly, you want to replace some controls with a custom renderer. But why would you add this special case to the default model?
I would argue that an adhoc solution is fine for those special cases.
Comment 7 Hallvard Traetteberg CLA 2018-06-14 08:08:52 EDT
(In reply to Eugen Neufeld from comment #6)
> It's not part of the core model, but so is not the VTable.
> The VAnnotation extends the VAttachment so you can always add it.

I found it now, in the org.eclipse.emfforms.view.annotation.model plugin.

> If I understand your use case correctly, you want to replace some controls
> with a custom renderer. But why would you add this special case to the
> default model?
> I would argue that an adhoc solution is fine for those special cases.

I don't consider it a special case, using a dropdown with Yes/No or other labels instead of a checkbox is a common case. In general, for any attribute type, there are multiple alternative widgets that in many cases are better than the default one, and it cannot be decided just based on the data type. Hence, it makes sense to let the designer provides hints for selecting the desired one, without requiring you to code it in Java as a service. By attaching a VAnnotation with a key/values pairs that can be picked up by a generic service, you can allow that service to select the corresponding renderer.

E.g. my renderer is named BooleanComboViewerSWTRenderer and my corresponding EMFFormsDIRendererService implementation checks that the attribute type is EBoolean, and if there is an attached VAnnotation with a "dropdown" value for the "style" key, then is returns a high priority. The BooleanComboViewerSWTRenderer can be configured with two labels corresponding to true and false, so it looks for the "labels" key and sets the labels according to the value.

I can easily imagine lots of renderers for attribute types that could be handled in the same way. One renderer is the default, but all can look for a "style" attribute (or similar) to increase the priority and all can be configured using other keys. If too many renderers use this technique, it will become a mess, since the interpretation of the keys and values aren't clear.

The alternative is to create a VAttachment subclass with everything needed for configuring your particular renderer, if an instance of that subclass is available, the corresponding renderer should be given priority and also be configured by it. This is essentially what I suggested, but since I didn't know about VAttachments I proposed something else. I'm not sure if the VAttachment list is the best place to add such info. but it should work.
Comment 8 Hallvard Traetteberg CLA 2018-06-14 09:32:15 EDT
Here are snippets that hopefully illustrate the idea:
 
(In reply to Hallvard Traetteberg from comment #7)
> E.g. my renderer is named BooleanComboViewerSWTRenderer and my corresponding
> EMFFormsDIRendererService implementation checks that the attribute type is
> EBoolean, and if there is an attached VAnnotation with a "dropdown" value
> for the "style" key, then is returns a high priority. The
> BooleanComboViewerSWTRenderer can be configured with two labels
> corresponding to true and false, so it looks for the "labels" key and sets
> the labels according to the value.

In EMFFormsDIRendererService impl:
if ("dropdown".equals(getAnnotationValue(vElement, "style", null))) {
   return 10;
}

In BooleanComboViewerSWTRenderer:
String labels = getAnnotationValue(vElement, "labels", null);
if (labels != null) {
   setTrueFalseLabels(labels.split(" "));
}

> I can easily imagine lots of renderers for attribute types that could be
> handled in the same way. One renderer is the default, but all can look for a
> "style" attribute (or similar) to increase the priority and all can be
> configured using other keys. If too many renderers use this technique, it
> will become a mess, since the interpretation of the keys and values aren't
> clear.
> 
> The alternative is to create a VAttachment subclass with everything needed
> for configuring your particular renderer, if an instance of that subclass is
> available, the corresponding renderer should be given priority and also be
> configured by it. This is essentially what I suggested, but since I didn't
> know about VAttachments I proposed something else. I'm not sure if the
> VAttachment list is the best place to add such info. but it should work.

With a VAttachment subclass, the code becomes:

In EMFFormsDIRendererService impl:
// check to see of there is a BooleanComboRendererConfig instance in the attachment list
if (getAttachment(vElement, BooleanComboRendererConfig.class) != null) {
   return 10;
}

In BooleanComboViewerSWTRenderer:
BooleanComboRendererConfig rendererConfig = getAttachment(vElement, BooleanComboRendererConfig.class);
if (rendererConfig != null) {
   setTrueFalseLabels(rendererConfig.getTrueFalseLabels());
}
Comment 9 Eugen Neufeld CLA 2018-08-22 09:35:44 EDT
Mass Move due to time constraints
Comment 10 Eugen Neufeld CLA 2018-11-06 04:40:59 EST
Mass move to 1.20 due to time constraints.
Comment 11 Eugen Neufeld CLA 2019-02-19 09:44:51 EST
I see your point, but we didn't encounter such cases in real world applications. Usually each application has its own style, eg using a checkbox for true/false or dropdowns. It rarely happens, that you come across different styles of the same type in the same application.
If you have such an application you can always either subclass the VControl element and provide a custom view model, introduce a custom attachment or any other solution which fits your requirements the best.

The lack of known use cases in real world applications and the possibility to extend EMFForms to your needs is the main reason for me to actively work on such a feature. 
Nonetheless we would happily accept any contribution which supports this use cases.
Comment 12 Eugen Neufeld CLA 2019-06-14 05:46:45 EDT
Please create custom view model elements which corresponding renderers or use attachments with custom testers.