On 18.06.2015 23:48, Michaël Michaud wrote:
> Le 18/06/2015 11:14, edgar.sol...@web.de a écrit :
>> On 18.06.2015 07:54, Michaël Michaud wrote:
>>>>> @@ -667,7 +668,11 @@
>>>>>           public void fireSelectionChanged() {
>>>>>                   for (Iterator i = listeners.iterator(); i.hasNext();) {
>>>>>                           LayerViewPanelListener l = 
>>>>> (LayerViewPanelListener) i.next();
>>>>> -                 l.selectionChanged();
>>>>> +            // [mmichaud 2015-06-17] Do not propagate selectionChange if 
>>>>> the change
>>>>> +            // comes from AttributeTablePanel
>>>>> +            if (getWorkBenchFrame().getActiveInternalFrame() instanceof 
>>>>> ViewAttributesPlugIn.ViewAttributesFrame) continue;
>>>>> +            if (getWorkBenchFrame().getActiveInternalFrame() instanceof 
>>>>> InfoFrame) continue;
>>>>> +            l.selectionChanged();
>>>>>                   }
>>>>>           }
>>>> hey Mike,
>>>>
>>>> sorry, but that's unclean. there may be other listeners (apart form 
>>>> Info-,Attrib.Frame) that will need but do not get notified this way.
>>> Right,
>>>
>>> What about r4501 ?
>>>
>> you moved it to the receiving end. that will probably work, but in theory a 
>> user could lightning-fast switch the active window and your test would fail.
> in case of lightning-fast switch, I think the following
> selectionModel.setFireSelectionReplaced(false);
> will still prevent entering an infinite loop which is my main concern.

where exactly is that or would you insert it?

>> we should probably go back a bit to the drawing board here. (mis)using 
>> selection event handlers for synchronization seems wrong. especially when 
>> they themselves start firing selection events.
>>
>> how about an approach like:
>> - layerview and attrib.table each keep their own logic to select features, 
>> when they are done they fire a section changed event
>> - both register 'special' selection changed listeners to each other which 
>> are merely updating their views, but do not actually change the selection, 
>> that was already done by the events cause.
> I'm not convinced. I don't defend my implementation which has probably its 
> flaws, but I can't see exactly how your proposition will solve the problem, 
> how complex it is and how much work it will be to implement it. Making 
> components listen to each other directly seems overcomplicated in case of 
> more components (currently, one can already sync the view, the table and the 
> info panels). Or maybe I missed your point.

it might seem more complicated, but it is
A. the architecture of OJ's original authors and difficult to replace with 
another approach
B. much cleaner and hence better maintainable as there is no "surprising" code

>> this is of course a real time approach, which is only necessary if users 
>> work with attrib table and layerview side by side and would expect them 
>> always to represent identical synched infos. you should test how that works 
>> for real big datasets, it might become slow there iterating over all 
>> features updating the views on each event.
> It is important to have layerview updated as soon as the table view is 
> updated. The opposite is probably less important, but
>> the faster (but not real time) option would be a synch on frame activation 
>> of course.
> having view synchronized only wen he window is activated is not really 
> satisfactory.

no objection here. simply saying it might prove slow on really big datasets.

>> additionally: 
>>  looks like you also integrated the update of layer's features 
>> (added,removed?) of the attrib.table on the selection events (ca. line 550 
>> AttributeTablePanel.java). you should probably attach listners to the 
>> attrib.tables layer for that purpose or agn. refresh on frame activation.
> Can you elaborate. I can't see anything special about adding removing layer's 
> feature in line 550.

it is the code path for updating the selection. as features inserting/deleting 
is unrelated to the current selected items i would argue it has no place there 
but to hacked in an extra routine, eg. the said layer change listener.

> On the other hand, I noticed that right-click delete is no more accessibe 
> from the table. Is that related to what you say ?
> 

don't know. didn't check.

..ede

------------------------------------------------------------------------------
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel

Reply via email to