[REVIEW REQUEST] RT-33442: isSelected in TableViewSelectionModel is called too many times
Stephen F Northover
steve.x.northover at oracle.com
Mon Oct 21 14:48:32 PDT 2013
On 2013-10-21 5:46 PM, Jonathan Giles wrote:
> I've added an updated patch to RT-33442. From the jira comment
> included with the patch:
> 1) selectRange method in TableSelectionModel is now abstract (given
> that TableSelectionModel is new in 8.0 this is fine).
> 2) selectRange argument list is now (int, TableColumnBase, int,
> 3) TableViewSelectionModel API is now unfortunately the same (i.e. I
> can't make the type TableColumn, which is more specific, and requires
> casting inside the method)
> 4) I have ported the patch to also apply to TreeTableView.
> 5) The same problem in 3) also exists for TreeTableView.
> I'll get a implementation review done by a controls team member, but a
> +1 on the API choice would be appreciated.
> -- Jonathan
> On 22/10/2013 9:43 a.m., Stephen F Northover wrote:
>> TableSelectionModel is new for JDK8 (at least according to the
>> Javadoc). If so, add the method as abstract and put implementations
>> in the concrete subclasses, no?
>> On 2013-10-21 3:15 PM, Jonathan Giles wrote:
>>> On 22/10/2013 2:35 a.m., Stephen F Northover wrote:
>>>> 1) Is it possible to do the optimization without adding API (ugly
>>>> but safe)?
>>> There are aspects of the optimisation that can be done without
>>> adding API. However, in my testing this API change is the most
>>> important of the changes, so without it there would be some, but
>>> much less, benefit of including the other changes.
>>>> 2) Another alternative (ugly) is to add the API but make it return
>>>> a boolean indicating whether it happened or not.
>>> Sure, I'm fine with this if this is the desired approach.
>>>> 3) It seem really weird to me that you can't convert from Column to
>>>> index and back again. What about getColumns()?
>>> The reason why this is the case is that TableSelectionModel is
>>> control independent, so it does not have the TableView or
>>> TreeTableView available to it. If it did these translations would be
>>> trivial (and are done all the time). This is why TableSelectionModel
>>> is currently a no-op, whereas the actual TableSelectionModel
>>> implementations (hidden within TableView and TreeTableView) can do
>>> precisely this.
>>> -- Jonathan
More information about the openjfx-dev