Problem with OpAction property value checks

[fgdrf]

Some parts of this message have been removed. Learn more about Nabble's security policy.

Hello List,

I run into some problems i guess everybody could run into and i ask you: is it a feature or maybe an issue? the effect can described as follows:

- i created an operation, used the net.refractions.udig.ui.operation extension point to get an Action for the targetClass net.refractions.udig.project.ILayer
- subclassed AbstractPropertyValue and created my own PropertyValue class (checks preferences for layer using a preferences store)
- the user select a layer, changed the preference for the selected layer and as a result an operation should be available within the context menu of the still selected layer
BUT the action (OpAction) is not visible (PropertyValue would return true) because the selection didn't changed.
If the customer deselects the layer and selects it again it works.

What I'm doing wrong? In my opinion the OpAction.updateEnablement method should not check whether the internal selection changed (see attached file OpAction.patch). But it is possible to run into performance issues if PropertyValue implementations are very heavy.

Could somebody comment my thoughts? If required I can create an issue for this.

Frank

_______________________________________________
User-friendly Desktop Internet GIS (uDig)
http://udig.refractions.net
http://lists.refractions.net/mailman/listinfo/udig-devel

[fgdrf]

Hello again,

just found a second check within the OpAction (inner class SetEnablement) and attached the new patch.

2009/10/29 Frank Gasdorf <fgdrf@...>

Hello List,

I run into some problems i guess everybody could run into and i ask you: is it a feature or maybe an issue? the effect can described as follows:

- i created an operation, used the net.refractions.udig.ui.operation extension point to get an Action for the targetClass net.refractions.udig.project.ILayer
- subclassed AbstractPropertyValue and created my own PropertyValue class (checks preferences for layer using a preferences store)
- the user select a layer, changed the preference for the selected layer and as a result an operation should be available within the context menu of the still selected layer
BUT the action (OpAction) is not visible (PropertyValue would return true) because the selection didn't changed.
If the customer deselects the layer and selects it again it works.

What I'm doing wrong? In my opinion the OpAction.updateEnablement method should not check whether the internal selection changed (see attached file OpAction.patch). But it is possible to run into performance issues if PropertyValue implementations are very heavy.

Could somebody comment my thoughts? If required I can create an issue for this.

Frank

_______________________________________________
User-friendly Desktop Internet GIS (uDig)
http://udig.refractions.net
http://lists.refractions.net/mailman/listinfo/udig-devel

[Jody Garnett-3]

> - i created an operation, used the net.refractions.udig.ui.operation
> extension point to get an Action for the targetClass
> net.refractions.udig.project.ILayer

Okay- so your operation should work on a Layer; or anything that can
adapt to a Layer - so probably a Map as well (assuming the current
layer).

> - subclassed AbstractPropertyValue and created my own PropertyValue class
> (checks preferences for layer using a preferences store)

> - the user select a layer, changed the preference for the selected layer and
> as a result an operation should be available within the context menu of the
> still selected layer
> BUT the action (OpAction) is not visible (PropertyValue would return true)
> because the selection didn't changed.
> If the customer deselects the layer and selects it again it works.

Oh I see; so you somehow want to ask it to check again. I am pretty
sure it only wants to check once for speed (rather then making the
right click operation slow). I wonder if the property value could
listen to the Layer? Or would that also be slow.


> What I'm doing wrong? In my opinion the OpAction.updateEnablement method
> should not check whether the internal selection changed (see attached file
> OpAction.patch). But it is possible to run into performance issues if
> PropertyValue implementations are very heavy.

Agreed.

> Could somebody comment my thoughts? If required I can create an issue for this.

I would like to create an issue for this so that:
a) we can talk to Jesse about it (who write the PropertyValue code)
b) work on a patch for his review

Jody
_______________________________________________
User-friendly Desktop Internet GIS (uDig)
http://udig.refractions.net
http://lists.refractions.net/mailman/listinfo/udig-devel

[fgdrf]

2009/10/30 Jody Garnett <jody.garnett@...>

> - i created an operation, used the net.refractions.udig.ui.operation
> extension point to get an Action for the targetClass
> net.refractions.udig.project.ILayer

Okay- so your operation should work on a Layer; or anything that can
adapt to a Layer - so probably a Map as well (assuming the current
layer).

Right.

> - subclassed AbstractPropertyValue and created my own PropertyValue class
> (checks preferences for layer using a preferences store)

> - the user select a layer, changed the preference for the selected layer and
> as a result an operation should be available within the context menu of the
> still selected layer
> BUT the action (OpAction) is not visible (PropertyValue would return true)
> because the selection didn't changed.
> If the customer deselects the layer and selects it again it works.

Oh I see; so you somehow want to ask it to check again. I am pretty
sure it only wants to check once for speed (rather then making the
right click operation slow). I wonder if the property value could
listen to the Layer? Or would that also be slow.

I guess the property value could listen to the layer but the changes occurs not on the layer or layer related objects. It's in the PreferencesStore, updated from a StyleEditorPage. Or is it even possible to set some properties for a layer? So maybe the PropertyValue implementation could listen to the layer(-properties).

 

> What I'm doing wrong? In my opinion the OpAction.updateEnablement method
> should not check whether the internal selection changed (see attached file
> OpAction.patch). But it is possible to run into performance issues if
> PropertyValue implementations are very heavy.

Agreed.

> Could somebody comment my thoughts? If required I can create an issue for this.

I would like to create an issue for this so that:
a) we can talk to Jesse about it (who write the PropertyValue code)
b) work on a patch for his review

Jody

OK, i create an issue and upload a patch. Jody, have you looked into the appended? Do you thing it solves the problem? I've seen there are some implementations but not to many and all of them are not so expensive at runtime, are they? Thanks for comments so far.

Frank

_______________________________________________
User-friendly Desktop Internet GIS (uDig)
http://udig.refractions.net
http://lists.refractions.net/mailman/listinfo/udig-devel