Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 335827

Summary:

Annotations for Action/Selection/Focus events

Product:

[RT] Riena

Reporter:

Holger Hoch <holger.hoch>

Component:

Assignee:

Stefan Liebig <Stefan.Liebig>

Status:

RESOLVED FIXED

QA Contact:

Severity:

enhancement

Priority:

CC:

christian.campo, Stefan.Liebig, stephan.mann

Version:

2.0.0

Target Milestone:

3.0.0.M7

Hardware:

All

OS:

All

Whiteboard:

Bug Depends on:

Bug Blocks:

335691

Attachments:

Description	Flags
Annotations for action and selection listeners	christian.campo: iplog+
OnActionCallbacks annotation	none

Description Holger Hoch

2011-01-31 05:42:13 EST

Build Identifier: 

This is a sub entry for bug 335691.
It provides annotations that may me used to mark an arbitrary controller method as a handler of a specific event fired by a specific ridget.
I.e. instead of writing controller code like this (example from o.e.r.e.c.c.ComboAndChoiceSubModuleController)

	updateAllRidgetsFromModel = getRidget("updateAllRidgetsFromModel");
	updateAllRidgetsFromModel.addListener(new IActionListener() {

		public void callback() {
			updateAllRidgetsFromModel();
		}
	});

within the #configureRidgets method you would just annotate an arbitrary Method (performing the same logic, of course) with the information upon which event on which ridget it should be executed:

	@HandlesActionCallback("updateAllRidgetsFromModel")
	public void handleUpdateAllRidgetsFromModel() {
		updateAllRidgetsFromModel();
	}

Note that the assignment of the ridget to an instance variable is not needed anymore.
The event to be handled is identified by the annotation type (@HandlesActionCallback) and the is identified by its name provided as the value parameter to the annotation.

The registration of the event handling methods takes place after the ridgets are injected into the IRidgetContainer but before the #afterBind method is called.
The methods must be public members of the IRidgetContainer containing the ridgets whose events are to be handled. The may optionally contain a single parameter which if given must be of the appropriate event type (eg o.e.r.u.r.l.SelectionEvent etc).

The patch contains the following annotations:
- @HandlesActionCallback
- @HandlesFocusGained
- @HandlesFocusLost
- @HandlesPropertyChange
- @HandlesSelectionChange
The @HandlesPropertyChange annotation is different in that it may require a second parameter to contrain event handling to a certain property:
@HandlesPropertyChange ("name", "text")
This second parameter is optional - if left out the event handler gets called on all property chanded events.

The known annotations are configured using an extension point, so you can add your own event handler anotations for your custom ridgets. The needs to be a corresponding handler implementation for each extension. Have a look at org.eclipse.riena.ui.ridgets.swt.uibinding.handler.ActionCallbackAnnotationHandler for an example.

The event handler registration is performed by the o.e.r.u.r.s.u.DefaultSwtBindingDelegate.

Please have a look at the implementation and share your thoughts with us.

Reproducible: Always

Comment 1 Holger Hoch

2011-01-31 05:47:58 EST

Created attachment 187946 [details]
Annotations for action and selection listeners

o.e.r.e.c.c.ComboAndChoiceSubModuleController and o.e.r.e.c.c.ListSubModuleController from the org.eclipse.riena.example.client plugin have been changed to use annotation for action and selection listeners

Comment 2 Stephan Mann

2011-02-10 07:39:41 EST

I fully second that enhancement! It would make controllers so much more readable and easier to develop.

Please consider improving the patch for the following use cases:
1. Provide a default annotation for double clicks in a list or table. AFAICS this is a common use case.
2. Provide a possibility to attach a listener to multiple ridgets. As an example, it might be useful to do some UI updates when one of multiple ridgets looses focus.

Comment 3 Stefan Liebig

2011-02-21 09:42:21 EST

in reply to comment #2

1) yes
2) If this can be done by annotating a 'listener' method with multiple annotations each with a different ridget id - would that be ok?

Comment 4 Stefan Liebig

2011-02-21 10:06:34 EST

General question:
How specific or how general shall annotation handlers be?
E.g. the handler responsible for 'addListener(IActionCallback)' performs the 'addListener' on just IActionRidgets. Shall it also perform that action on any kind of ridget supporting this signature? In this case also for ITraverseRidget?

Maybe use duck typing!?

Comment 5 Christian Campo

2011-02-21 10:23:56 EST

I am against duck-typing since that means using reflection. But I would be PRO having that annocation for IActionRidget and ITraverseRidget.

Comment 6 Stefan Liebig

2011-02-22 09:34:28 EST

Thanks Holger!

Patch applied with a few refactorings.

in reply to comment 4:
NO duck typing!

Comment 7 Stephan Mann

2011-03-24 08:35:13 EDT

I was just trying to use this new, very cool feature but sadly I can't use it for all use cases I was hoping for. As described in comment#2 I'd like to use it, among other things, to react to change events and trigger an UI update:

@OnPropertyChange(ridgetId = BINDING_ID_RIDGET1)
@OnPropertyChange(ridgetId = BINDING_ID_RIDGET2)
public void doUiRefresh() {
	[...]
}

I guess that's not possible but it would be nice to be at least able to provide a list of ridget IDs. Otherwise one would have to fall back to "the old ways" in this case. I guess the same goes for all other annotations.

Comment 8 Stefan Liebig

2011-03-24 08:37:38 EDT

Reopened because of comment #7

Comment 9 Stefan Liebig

2011-03-24 09:20:05 EDT

Yes, this is very sad. Unfortunately Java does not allow this.

However, there are common patterns (requiring additional coding) to solve such issues.

1. Nested annotations, e.g.
@OnPropertyChanges( {
    @OnPropertyChange(ridgetId = BINDING_ID_RIDGET1),
    @OnPropertyChange(ridgetId = BINDING_ID_RIDGET2) } )
public void doUiRefresh() {
    [...]
}

2. Allow multiple values (array) for ridgetId (would become ridgitIds)
@OnPropertyChange(ridgetIds ={ BINDING_ID_RIDGET1, BINDING_ID_RIDGET2 } )
public void doUiRefresh() {
    [...]
}

3. Allow the mutliple values be expressed within a single comma separated string
@OnPropertyChange(ridgetId = BINDING_ID_RIDGET1 + "," + BINDING_ID_RIDGET2 )
public void doUiRefresh() {
    [...]
}


I would go for solution 1. since this is the most flexibel and obvious solution.

Comment 10 Stephan Mann

2011-03-24 09:48:49 EDT

I too would prefer solution 1, since the other ones become unreadable if an additional parameter needs to be provided (e.g. propertyName for this @OnPropertyChange).

Comment 11 Christian Campo

2011-03-24 12:25:57 EDT

(In reply to comment #10)
> I too would prefer solution 1, since the other ones become unreadable if an
> additional parameter needs to be provided (e.g. propertyName for this
> @OnPropertyChange).

I could agree if we agree that the unnested version for one ridget continue to exist. That the 80 % use case and I would hate if I had to write a nested annotation for that simple use case….

Comment 12 Stefan Liebig

2011-03-24 14:40:48 EDT

(In reply to comment #11)

> I could agree if we agree that the unnested version for one ridget continue to
> exist. That the 80 % use case and I would hate if I had to write a nested
> annotation for that simple use case….

Of course, do they still exist! They are part of the outer "container" annotation. All of the "ridget"-event annotations will get a corresponding "container" annotation with the same name in plural.

Comment 13 Stefan Liebig

2011-03-24 16:20:58 EDT

Created attachment 191859 [details]
OnActionCallbacks annotation

Comment 14 Stefan Liebig

2011-03-28 09:09:05 EDT

Added annotations (suggestion 1)and handler as described in comment #9