You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pivot.apache.org by Greg Brown <gk...@mac.com> on 2010/06/24 21:34:19 UTC

[RFC] Selection change events

Hi all,

I am thinking about making a change to how selection change events are fired and I would like to hear your feedback. Currently, selection change events are fired only when an explicit call has been made that affects the selection. 

For example, in ListView, calling either setSelectedRanges() or clearSelection() will fire this event. However, an operation that indirectly changes the selection state (such as adding or removing an item from the ListView's model data) does not trigger an event. This was originally done by design - selectedRangesChanged() includes the previous selection as an argument, and we didn't want to have to manually re-construct that every time the selection changed as a side effect of a model change:

  public void selectedRangesChanged(ListView listView, Sequence<Span> previousSelectedRanges);

However, in practice, I have found this to be a bit challenging. More than once I have registered a selection change listener expecting to receive notification of all selection changes, forgetting that it is not designed that way. If I am getting tripped up by this, I'm guessing that other developers might be as well.

So, I am proposing that components that maintain a selection state also fire selection change events when the selection changes indirectly. In this case, a null value would be passed for the previous selection. This will save the effort of re-constructing the previous selection info and will give the listener additional information about the nature of the change (i.e. null == indirect state change).

Please let me know what you think.

Thanks for your input,
Greg



Re: [RFC] Selection change events

Posted by Greg Brown <gk...@mac.com>.
I think that the nice thing about this approach is that the API does remain consistent (the listener method still retains the previous selection argument), but we still save the performance hit when the event is fired due to an indirect change.

If no one has any additional concerns, I'll probably try to tackle this next week.


On Jun 25, 2010, at 7:20 PM, aappddeevv wrote:

> 
> This is a good point actually and actually I think the e4/jface breaks out
> selection into multiple firings in order to help with performance although
> in the post selection listener (the very last call) it does do an entire
> diff calculation-but its only once. So if we stick with just one firing for
> selection, the null may be more conservative. I guess this is balanced
> against consistency of the API though.
> 
> -----Original Message-----
> From: Greg Brown [mailto:gkbrown@mac.com] 
> Sent: Friday, June 25, 2010 9:30 AM
> To: dev@pivot.apache.org
> Subject: Re: [RFC] Selection change events
> 
> It would be a needless performance hit. Every time you add or remove an item
> from the model, we'd need to create a copy of the entire selection state
> just to include it in the event. Since, as you pointed out, the common case
> is probably to ignore the previous selection argument, this would be
> wasteful. As a developer, I'd rather deal with a potential null argument
> value than a performance hit that I can't avoid.
> 
> On Jun 25, 2010, at 9:24 AM, Noel Grandin wrote:
> 
>> Hi
>> 
>> OK, I see your case.
>> 
>> I'm just concerned about the "sometimes previousSelected is available,
> sometimes it's not" situation.
>> 
>> Perhaps we should rather just take the complexity knock and always work
> out the previous selection - what was the
>> problem here again?
>> 
>> -- Noel
>> 
>> Greg Brown wrote:
>>> Hi Noel,
>>> 
>>> I did think about removing that argument, but there are a few reasons why
> I didn't think it was the right approach:
>>> 
>>> 1) All of our other property change events carry a previous value, so
> this would create an inconsistency.
>>> 
>>> 2) I think there are potentially valid use cases for it. Let's say I have
> a list view containing 10 image file names. Next to the list view I have a
> grid view containing image thumbnails. When an image is selected, I want to
> set a background color for the corresponding image; when deselected, I want
> to clear it. It will be more efficient to implement this logic if I have
> access to the previous selected indexes. This is a contrived example but I'm
> sure that someone will run into a real-world use case for it.
>>> 
>>> 3) It is already supported and carries almost no overhead, so taking it
> out would be removing potentially useful functionality.
>>> 
>>> Another possibility is to create a new listener interface to fire this
> event, similar to TextInputTextListener, which defines textChanged() and
> does not include the previous value. However, we already have
> ListViewSelectionListener, so I'm not sure what we might call it. That's how
> I came to the idea of simply overloading the previous value argument.
>>> 
>>> This does make me realize that TextInput is also a bit inconsistent,
> since setText() fires textChanged() and does not pass the previous value. So
> it may actually make sense to consolidate TextInputCharacterListener and
> TextInputTextListener into a single interface and take a similar approach
> there.
>>> 
>>> G
>>> 
>>> On Jun 25, 2010, at 4:25 AM, Noel Grandin wrote:
>>> 
>>>> HI
>>>> 
>>>> I think your change is a good idea, but I don't really like using null
>>>> as an indicator of something special.
>>>> It sounds like an invitation to make mistakes about when the parameter
>>>> is or is not useful.
>>>> 
>>>> Why don't we just make this a special case and drop the "previous"
>>>> parameters?
>>>> I can't see many callers wanting to make use of it, and they could
>>>> always keep track of it themselves if they really need to.
>>>> 
>>>> -- Noel
>>>> 
>>>> Greg Brown wrote:
>>>>> Hi all,
>>>>> 
>>>>> I am thinking about making a change to how selection change events are
> fired and I would like to hear your feedback. Currently, selection change
> events are fired only when an explicit call has been made that affects the
> selection. 
>>>>> 
>>>>> For example, in ListView, calling either setSelectedRanges() or
> clearSelection() will fire this event. However, an operation that indirectly
> changes the selection state (such as adding or removing an item from the
> ListView's model data) does not trigger an event. This was originally done
> by design - selectedRangesChanged() includes the previous selection as an
> argument, and we didn't want to have to manually re-construct that every
> time the selection changed as a side effect of a model change:
>>>>> 
>>>>> public void selectedRangesChanged(ListView listView, Sequence<Span>
> previousSelectedRanges);
>>>>> 
>>>>> However, in practice, I have found this to be a bit challenging. More
> than once I have registered a selection change listener expecting to receive
> notification of all selection changes, forgetting that it is not designed
> that way. If I am getting tripped up by this, I'm guessing that other
> developers might be as well.
>>>>> 
>>>>> So, I am proposing that components that maintain a selection state also
> fire selection change events when the selection changes indirectly. In this
> case, a null value would be passed for the previous selection. This will
> save the effort of re-constructing the previous selection info and will give
> the listener additional information about the nature of the change (i.e.
> null == indirect state change).
>>>>> 
>>>>> Please let me know what you think.
>>>>> 
>>>>> Thanks for your input,
>>>>> Greg
>>>>> 
>>>>> 
>>>>> 
>> 
> 


RE: [RFC] Selection change events

Posted by aappddeevv <aa...@verizon.net>.
This is a good point actually and actually I think the e4/jface breaks out
selection into multiple firings in order to help with performance although
in the post selection listener (the very last call) it does do an entire
diff calculation-but its only once. So if we stick with just one firing for
selection, the null may be more conservative. I guess this is balanced
against consistency of the API though.

-----Original Message-----
From: Greg Brown [mailto:gkbrown@mac.com] 
Sent: Friday, June 25, 2010 9:30 AM
To: dev@pivot.apache.org
Subject: Re: [RFC] Selection change events

It would be a needless performance hit. Every time you add or remove an item
from the model, we'd need to create a copy of the entire selection state
just to include it in the event. Since, as you pointed out, the common case
is probably to ignore the previous selection argument, this would be
wasteful. As a developer, I'd rather deal with a potential null argument
value than a performance hit that I can't avoid.

On Jun 25, 2010, at 9:24 AM, Noel Grandin wrote:

> Hi
> 
> OK, I see your case.
> 
> I'm just concerned about the "sometimes previousSelected is available,
sometimes it's not" situation.
> 
> Perhaps we should rather just take the complexity knock and always work
out the previous selection - what was the
> problem here again?
> 
> -- Noel
> 
> Greg Brown wrote:
>> Hi Noel,
>> 
>> I did think about removing that argument, but there are a few reasons why
I didn't think it was the right approach:
>> 
>> 1) All of our other property change events carry a previous value, so
this would create an inconsistency.
>> 
>> 2) I think there are potentially valid use cases for it. Let's say I have
a list view containing 10 image file names. Next to the list view I have a
grid view containing image thumbnails. When an image is selected, I want to
set a background color for the corresponding image; when deselected, I want
to clear it. It will be more efficient to implement this logic if I have
access to the previous selected indexes. This is a contrived example but I'm
sure that someone will run into a real-world use case for it.
>> 
>> 3) It is already supported and carries almost no overhead, so taking it
out would be removing potentially useful functionality.
>> 
>> Another possibility is to create a new listener interface to fire this
event, similar to TextInputTextListener, which defines textChanged() and
does not include the previous value. However, we already have
ListViewSelectionListener, so I'm not sure what we might call it. That's how
I came to the idea of simply overloading the previous value argument.
>> 
>> This does make me realize that TextInput is also a bit inconsistent,
since setText() fires textChanged() and does not pass the previous value. So
it may actually make sense to consolidate TextInputCharacterListener and
TextInputTextListener into a single interface and take a similar approach
there.
>> 
>> G
>> 
>> On Jun 25, 2010, at 4:25 AM, Noel Grandin wrote:
>> 
>>> HI
>>> 
>>> I think your change is a good idea, but I don't really like using null
>>> as an indicator of something special.
>>> It sounds like an invitation to make mistakes about when the parameter
>>> is or is not useful.
>>> 
>>> Why don't we just make this a special case and drop the "previous"
>>> parameters?
>>> I can't see many callers wanting to make use of it, and they could
>>> always keep track of it themselves if they really need to.
>>> 
>>> -- Noel
>>> 
>>> Greg Brown wrote:
>>>> Hi all,
>>>> 
>>>> I am thinking about making a change to how selection change events are
fired and I would like to hear your feedback. Currently, selection change
events are fired only when an explicit call has been made that affects the
selection. 
>>>> 
>>>> For example, in ListView, calling either setSelectedRanges() or
clearSelection() will fire this event. However, an operation that indirectly
changes the selection state (such as adding or removing an item from the
ListView's model data) does not trigger an event. This was originally done
by design - selectedRangesChanged() includes the previous selection as an
argument, and we didn't want to have to manually re-construct that every
time the selection changed as a side effect of a model change:
>>>> 
>>>> public void selectedRangesChanged(ListView listView, Sequence<Span>
previousSelectedRanges);
>>>> 
>>>> However, in practice, I have found this to be a bit challenging. More
than once I have registered a selection change listener expecting to receive
notification of all selection changes, forgetting that it is not designed
that way. If I am getting tripped up by this, I'm guessing that other
developers might be as well.
>>>> 
>>>> So, I am proposing that components that maintain a selection state also
fire selection change events when the selection changes indirectly. In this
case, a null value would be passed for the previous selection. This will
save the effort of re-constructing the previous selection info and will give
the listener additional information about the nature of the change (i.e.
null == indirect state change).
>>>> 
>>>> Please let me know what you think.
>>>> 
>>>> Thanks for your input,
>>>> Greg
>>>> 
>>>> 
>>>> 
> 


Re: [RFC] Selection change events

Posted by Greg Brown <gk...@mac.com>.
It would be a needless performance hit. Every time you add or remove an item from the model, we'd need to create a copy of the entire selection state just to include it in the event. Since, as you pointed out, the common case is probably to ignore the previous selection argument, this would be wasteful. As a developer, I'd rather deal with a potential null argument value than a performance hit that I can't avoid.

On Jun 25, 2010, at 9:24 AM, Noel Grandin wrote:

> Hi
> 
> OK, I see your case.
> 
> I'm just concerned about the "sometimes previousSelected is available, sometimes it's not" situation.
> 
> Perhaps we should rather just take the complexity knock and always work out the previous selection - what was the
> problem here again?
> 
> -- Noel
> 
> Greg Brown wrote:
>> Hi Noel,
>> 
>> I did think about removing that argument, but there are a few reasons why I didn't think it was the right approach:
>> 
>> 1) All of our other property change events carry a previous value, so this would create an inconsistency.
>> 
>> 2) I think there are potentially valid use cases for it. Let's say I have a list view containing 10 image file names. Next to the list view I have a grid view containing image thumbnails. When an image is selected, I want to set a background color for the corresponding image; when deselected, I want to clear it. It will be more efficient to implement this logic if I have access to the previous selected indexes. This is a contrived example but I'm sure that someone will run into a real-world use case for it.
>> 
>> 3) It is already supported and carries almost no overhead, so taking it out would be removing potentially useful functionality.
>> 
>> Another possibility is to create a new listener interface to fire this event, similar to TextInputTextListener, which defines textChanged() and does not include the previous value. However, we already have ListViewSelectionListener, so I'm not sure what we might call it. That's how I came to the idea of simply overloading the previous value argument.
>> 
>> This does make me realize that TextInput is also a bit inconsistent, since setText() fires textChanged() and does not pass the previous value. So it may actually make sense to consolidate TextInputCharacterListener and TextInputTextListener into a single interface and take a similar approach there.
>> 
>> G
>> 
>> On Jun 25, 2010, at 4:25 AM, Noel Grandin wrote:
>> 
>>> HI
>>> 
>>> I think your change is a good idea, but I don't really like using null
>>> as an indicator of something special.
>>> It sounds like an invitation to make mistakes about when the parameter
>>> is or is not useful.
>>> 
>>> Why don't we just make this a special case and drop the "previous"
>>> parameters?
>>> I can't see many callers wanting to make use of it, and they could
>>> always keep track of it themselves if they really need to.
>>> 
>>> -- Noel
>>> 
>>> Greg Brown wrote:
>>>> Hi all,
>>>> 
>>>> I am thinking about making a change to how selection change events are fired and I would like to hear your feedback. Currently, selection change events are fired only when an explicit call has been made that affects the selection. 
>>>> 
>>>> For example, in ListView, calling either setSelectedRanges() or clearSelection() will fire this event. However, an operation that indirectly changes the selection state (such as adding or removing an item from the ListView's model data) does not trigger an event. This was originally done by design - selectedRangesChanged() includes the previous selection as an argument, and we didn't want to have to manually re-construct that every time the selection changed as a side effect of a model change:
>>>> 
>>>> public void selectedRangesChanged(ListView listView, Sequence<Span> previousSelectedRanges);
>>>> 
>>>> However, in practice, I have found this to be a bit challenging. More than once I have registered a selection change listener expecting to receive notification of all selection changes, forgetting that it is not designed that way. If I am getting tripped up by this, I'm guessing that other developers might be as well.
>>>> 
>>>> So, I am proposing that components that maintain a selection state also fire selection change events when the selection changes indirectly. In this case, a null value would be passed for the previous selection. This will save the effort of re-constructing the previous selection info and will give the listener additional information about the nature of the change (i.e. null == indirect state change).
>>>> 
>>>> Please let me know what you think.
>>>> 
>>>> Thanks for your input,
>>>> Greg
>>>> 
>>>> 
>>>> 
> 


Re: [RFC] Selection change events

Posted by Noel Grandin <no...@gmail.com>.
 Hi

OK, I see your case.

I'm just concerned about the "sometimes previousSelected is available, sometimes it's not" situation.

Perhaps we should rather just take the complexity knock and always work out the previous selection - what was the
problem here again?

-- Noel

Greg Brown wrote:
> Hi Noel,
>
> I did think about removing that argument, but there are a few reasons why I didn't think it was the right approach:
>
> 1) All of our other property change events carry a previous value, so this would create an inconsistency.
>
> 2) I think there are potentially valid use cases for it. Let's say I have a list view containing 10 image file names. Next to the list view I have a grid view containing image thumbnails. When an image is selected, I want to set a background color for the corresponding image; when deselected, I want to clear it. It will be more efficient to implement this logic if I have access to the previous selected indexes. This is a contrived example but I'm sure that someone will run into a real-world use case for it.
>
> 3) It is already supported and carries almost no overhead, so taking it out would be removing potentially useful functionality.
>
> Another possibility is to create a new listener interface to fire this event, similar to TextInputTextListener, which defines textChanged() and does not include the previous value. However, we already have ListViewSelectionListener, so I'm not sure what we might call it. That's how I came to the idea of simply overloading the previous value argument.
>
> This does make me realize that TextInput is also a bit inconsistent, since setText() fires textChanged() and does not pass the previous value. So it may actually make sense to consolidate TextInputCharacterListener and TextInputTextListener into a single interface and take a similar approach there.
>
> G
>
> On Jun 25, 2010, at 4:25 AM, Noel Grandin wrote:
>
>> HI
>>
>> I think your change is a good idea, but I don't really like using null
>> as an indicator of something special.
>> It sounds like an invitation to make mistakes about when the parameter
>> is or is not useful.
>>
>> Why don't we just make this a special case and drop the "previous"
>> parameters?
>> I can't see many callers wanting to make use of it, and they could
>> always keep track of it themselves if they really need to.
>>
>> -- Noel
>>
>> Greg Brown wrote:
>>> Hi all,
>>>
>>> I am thinking about making a change to how selection change events are fired and I would like to hear your feedback. Currently, selection change events are fired only when an explicit call has been made that affects the selection. 
>>>
>>> For example, in ListView, calling either setSelectedRanges() or clearSelection() will fire this event. However, an operation that indirectly changes the selection state (such as adding or removing an item from the ListView's model data) does not trigger an event. This was originally done by design - selectedRangesChanged() includes the previous selection as an argument, and we didn't want to have to manually re-construct that every time the selection changed as a side effect of a model change:
>>>
>>>  public void selectedRangesChanged(ListView listView, Sequence<Span> previousSelectedRanges);
>>>
>>> However, in practice, I have found this to be a bit challenging. More than once I have registered a selection change listener expecting to receive notification of all selection changes, forgetting that it is not designed that way. If I am getting tripped up by this, I'm guessing that other developers might be as well.
>>>
>>> So, I am proposing that components that maintain a selection state also fire selection change events when the selection changes indirectly. In this case, a null value would be passed for the previous selection. This will save the effort of re-constructing the previous selection info and will give the listener additional information about the nature of the change (i.e. null == indirect state change).
>>>
>>> Please let me know what you think.
>>>
>>> Thanks for your input,
>>> Greg
>>>
>>>
>>>


Re: [RFC] Selection change events

Posted by Greg Brown <gk...@mac.com>.
Hi Noel,

I did think about removing that argument, but there are a few reasons why I didn't think it was the right approach:

1) All of our other property change events carry a previous value, so this would create an inconsistency.

2) I think there are potentially valid use cases for it. Let's say I have a list view containing 10 image file names. Next to the list view I have a grid view containing image thumbnails. When an image is selected, I want to set a background color for the corresponding image; when deselected, I want to clear it. It will be more efficient to implement this logic if I have access to the previous selected indexes. This is a contrived example but I'm sure that someone will run into a real-world use case for it.

3) It is already supported and carries almost no overhead, so taking it out would be removing potentially useful functionality.

Another possibility is to create a new listener interface to fire this event, similar to TextInputTextListener, which defines textChanged() and does not include the previous value. However, we already have ListViewSelectionListener, so I'm not sure what we might call it. That's how I came to the idea of simply overloading the previous value argument.

This does make me realize that TextInput is also a bit inconsistent, since setText() fires textChanged() and does not pass the previous value. So it may actually make sense to consolidate TextInputCharacterListener and TextInputTextListener into a single interface and take a similar approach there.

G

On Jun 25, 2010, at 4:25 AM, Noel Grandin wrote:

> HI
> 
> I think your change is a good idea, but I don't really like using null
> as an indicator of something special.
> It sounds like an invitation to make mistakes about when the parameter
> is or is not useful.
> 
> Why don't we just make this a special case and drop the "previous"
> parameters?
> I can't see many callers wanting to make use of it, and they could
> always keep track of it themselves if they really need to.
> 
> -- Noel
> 
> Greg Brown wrote:
>> Hi all,
>> 
>> I am thinking about making a change to how selection change events are fired and I would like to hear your feedback. Currently, selection change events are fired only when an explicit call has been made that affects the selection. 
>> 
>> For example, in ListView, calling either setSelectedRanges() or clearSelection() will fire this event. However, an operation that indirectly changes the selection state (such as adding or removing an item from the ListView's model data) does not trigger an event. This was originally done by design - selectedRangesChanged() includes the previous selection as an argument, and we didn't want to have to manually re-construct that every time the selection changed as a side effect of a model change:
>> 
>>  public void selectedRangesChanged(ListView listView, Sequence<Span> previousSelectedRanges);
>> 
>> However, in practice, I have found this to be a bit challenging. More than once I have registered a selection change listener expecting to receive notification of all selection changes, forgetting that it is not designed that way. If I am getting tripped up by this, I'm guessing that other developers might be as well.
>> 
>> So, I am proposing that components that maintain a selection state also fire selection change events when the selection changes indirectly. In this case, a null value would be passed for the previous selection. This will save the effort of re-constructing the previous selection info and will give the listener additional information about the nature of the change (i.e. null == indirect state change).
>> 
>> Please let me know what you think.
>> 
>> Thanks for your input,
>> Greg
>> 
>> 
>> 
> 


Re: [RFC] Selection change events

Posted by Noel Grandin <no...@gmail.com>.
HI

I think your change is a good idea, but I don't really like using null
as an indicator of something special.
It sounds like an invitation to make mistakes about when the parameter
is or is not useful.

Why don't we just make this a special case and drop the "previous"
parameters?
I can't see many callers wanting to make use of it, and they could
always keep track of it themselves if they really need to.

-- Noel

Greg Brown wrote:
> Hi all,
>
> I am thinking about making a change to how selection change events are fired and I would like to hear your feedback. Currently, selection change events are fired only when an explicit call has been made that affects the selection. 
>
> For example, in ListView, calling either setSelectedRanges() or clearSelection() will fire this event. However, an operation that indirectly changes the selection state (such as adding or removing an item from the ListView's model data) does not trigger an event. This was originally done by design - selectedRangesChanged() includes the previous selection as an argument, and we didn't want to have to manually re-construct that every time the selection changed as a side effect of a model change:
>
>   public void selectedRangesChanged(ListView listView, Sequence<Span> previousSelectedRanges);
>
> However, in practice, I have found this to be a bit challenging. More than once I have registered a selection change listener expecting to receive notification of all selection changes, forgetting that it is not designed that way. If I am getting tripped up by this, I'm guessing that other developers might be as well.
>
> So, I am proposing that components that maintain a selection state also fire selection change events when the selection changes indirectly. In this case, a null value would be passed for the previous selection. This will save the effort of re-constructing the previous selection info and will give the listener additional information about the nature of the change (i.e. null == indirect state change).
>
> Please let me know what you think.
>
> Thanks for your input,
> Greg
>
>
>   


Re: [RFC] Selection change events

Posted by Greg Brown <gk...@mac.com>.
When selection changes directly, we simply pass the old selection state to the listener, since there is basically no overhead in doing so. When it changes indirectly, we'd have to manually create the previous selection, which is an unnecessary performance hit.

On Jun 24, 2010, at 10:12 PM, aappddeevv wrote:

> This sounds good. But why would you pass null? Should it not be be the
> previous selection info (the current selection prior to changing it
> indirectly? This would be consistent with doing it "directly." 
> -----Original Message-----
> From: Greg Brown [mailto:gkbrown@mac.com] 
> Sent: Thursday, June 24, 2010 3:34 PM
> To: dev@pivot.apache.org
> Subject: [RFC] Selection change events
> 
> Hi all,
> 
> I am thinking about making a change to how selection change events are fired
> and I would like to hear your feedback. Currently, selection change events
> are fired only when an explicit call has been made that affects the
> selection. 
> 
> For example, in ListView, calling either setSelectedRanges() or
> clearSelection() will fire this event. However, an operation that indirectly
> changes the selection state (such as adding or removing an item from the
> ListView's model data) does not trigger an event. This was originally done
> by design - selectedRangesChanged() includes the previous selection as an
> argument, and we didn't want to have to manually re-construct that every
> time the selection changed as a side effect of a model change:
> 
>  public void selectedRangesChanged(ListView listView, Sequence<Span>
> previousSelectedRanges);
> 
> However, in practice, I have found this to be a bit challenging. More than
> once I have registered a selection change listener expecting to receive
> notification of all selection changes, forgetting that it is not designed
> that way. If I am getting tripped up by this, I'm guessing that other
> developers might be as well.
> 
> So, I am proposing that components that maintain a selection state also fire
> selection change events when the selection changes indirectly. In this case,
> a null value would be passed for the previous selection. This will save the
> effort of re-constructing the previous selection info and will give the
> listener additional information about the nature of the change (i.e. null ==
> indirect state change).
> 
> Please let me know what you think.
> 
> Thanks for your input,
> Greg
> 
> 


RE: [RFC] Selection change events

Posted by aappddeevv <aa...@verizon.net>.
This sounds good. But why would you pass null? Should it not be be the
previous selection info (the current selection prior to changing it
indirectly? This would be consistent with doing it "directly." 
-----Original Message-----
From: Greg Brown [mailto:gkbrown@mac.com] 
Sent: Thursday, June 24, 2010 3:34 PM
To: dev@pivot.apache.org
Subject: [RFC] Selection change events

Hi all,

I am thinking about making a change to how selection change events are fired
and I would like to hear your feedback. Currently, selection change events
are fired only when an explicit call has been made that affects the
selection. 

For example, in ListView, calling either setSelectedRanges() or
clearSelection() will fire this event. However, an operation that indirectly
changes the selection state (such as adding or removing an item from the
ListView's model data) does not trigger an event. This was originally done
by design - selectedRangesChanged() includes the previous selection as an
argument, and we didn't want to have to manually re-construct that every
time the selection changed as a side effect of a model change:

  public void selectedRangesChanged(ListView listView, Sequence<Span>
previousSelectedRanges);

However, in practice, I have found this to be a bit challenging. More than
once I have registered a selection change listener expecting to receive
notification of all selection changes, forgetting that it is not designed
that way. If I am getting tripped up by this, I'm guessing that other
developers might be as well.

So, I am proposing that components that maintain a selection state also fire
selection change events when the selection changes indirectly. In this case,
a null value would be passed for the previous selection. This will save the
effort of re-constructing the previous selection info and will give the
listener additional information about the nature of the change (i.e. null ==
indirect state change).

Please let me know what you think.

Thanks for your input,
Greg



Re: [RFC] Selection change events

Posted by Greg Brown <gk...@mac.com>.
It depends on how the selection is set. If you call setSelectedRanges(), a single event is fired for the entire change. If you call addSelectedRange() or removeSelectedRange(), an event is fired for each change.

On Jun 24, 2010, at 10:18 PM, aappddeevv wrote:

> I'm not sure of the current implementation, but if by doing a selection
> indirectly, you fire an event for each selection, as it is selected in the
> set being set, then that could be inefficient or at least push the burden of
> inefficiency into the client. If it fires after all of them have been
> selected, then it should be okay. I think some platforms, can't recall
> immediately, allow you to receive each individual selection, if you want it,
> then a post selection event that has all of them. It probably uses a timer
> underneath to determine which selections get grouped together.
> 
> 
> -----Original Message-----
> From: Sandro Martini [mailto:sandro.martini@gmail.com] 
> Sent: Thursday, June 24, 2010 5:53 PM
> To: dev@pivot.apache.org
> Subject: Re: [RFC] Selection change events
> 
> Hi Greg,
> for me the change is right, and maybe with this application selection
> listeners could see what to do (and when it's the case ignoring the
> "indirect change" event, like today) ... and on all selectable
> components, right ?
> Have you an idea on the performance impact of this change on
> tables/trees and other containers with many elements inside ?
> 
> Bye,
> Sandro
> 


RE: [RFC] Selection change events

Posted by aappddeevv <aa...@verizon.net>.
I'm not sure of the current implementation, but if by doing a selection
indirectly, you fire an event for each selection, as it is selected in the
set being set, then that could be inefficient or at least push the burden of
inefficiency into the client. If it fires after all of them have been
selected, then it should be okay. I think some platforms, can't recall
immediately, allow you to receive each individual selection, if you want it,
then a post selection event that has all of them. It probably uses a timer
underneath to determine which selections get grouped together.


-----Original Message-----
From: Sandro Martini [mailto:sandro.martini@gmail.com] 
Sent: Thursday, June 24, 2010 5:53 PM
To: dev@pivot.apache.org
Subject: Re: [RFC] Selection change events

Hi Greg,
for me the change is right, and maybe with this application selection
listeners could see what to do (and when it's the case ignoring the
"indirect change" event, like today) ... and on all selectable
components, right ?
Have you an idea on the performance impact of this change on
tables/trees and other containers with many elements inside ?

Bye,
Sandro


Re: [RFC] Selection change events

Posted by Sandro Martini <sa...@gmail.com>.
> The performance impact should be negligible.
I was sure of it :-) ...

Re: [RFC] Selection change events

Posted by Greg Brown <gk...@mac.com>.
The performance impact should be negligible.

On Jun 24, 2010, at 5:52 PM, Sandro Martini wrote:

> Hi Greg,
> for me the change is right, and maybe with this application selection
> listeners could see what to do (and when it's the case ignoring the
> "indirect change" event, like today) ... and on all selectable
> components, right ?
> Have you an idea on the performance impact of this change on
> tables/trees and other containers with many elements inside ?
> 
> Bye,
> Sandro


Re: [RFC] Selection change events

Posted by Sandro Martini <sa...@gmail.com>.
Hi Greg,
for me the change is right, and maybe with this application selection
listeners could see what to do (and when it's the case ignoring the
"indirect change" event, like today) ... and on all selectable
components, right ?
Have you an idea on the performance impact of this change on
tables/trees and other containers with many elements inside ?

Bye,
Sandro