CPTableView always sends tableViewSelectionDidChange #1375

Open
kuon opened this Issue Oct 7, 2011 · 19 comments

Comments

Projects
None yet
8 participants
Contributor

kuon commented Oct 7, 2011

In the documentation of the delegate methods of CPTableView, it is stated that tableViewSelectionDidChange: is triggered only by user action (clicking the table). But in reality, tableViewSelectionDidChange: is sent to the delegate for all selection change like selectRowIndexes:byExtendingSelection:.

We should either update the documentation, or, more appropriately, change the behaviour for the delegate to be notified only when the selection is modified by user action.

I am getting the same thing as well. In the below sample code, tableViewSelectionIsChanging does not fire with mouse events (using the arrow key to navigate the table) but tableViewSelectionDidChange does.

https://gist.github.com/1368716

Contributor

Me1000 commented Dec 13, 2011

Id like to know what you all think of this. The cocoa behavior is as we documented. The behavior is a regression. But, I think updating the documentation may be more useful.

You're right! I would push for better documentation on cappuccino's side. Either removing all definitions of table selection or adding just a bit to give an accurate behavior. The current capp docs have:

tableViewSelectionIsChanging - Informs the delegate that the tableview selection has changed.
tableViewSelectionDidChange - Blank

Hope you can see why I jumped to a Bug! I think the adding just a bit more would clear it all up:

tableViewSelectionIsChanging - Informs the delegate that the tableview selection is about to change. This is only sent with mouse click events.

tableViewSelectionDidChange - Informs the delegate that the tableview selection has changed.

Contributor

kuon commented Dec 13, 2011

I think we should not be notified when we change the selection programatically, because it makes altering the selection programatically very expensive, and when you update the selection programatically, you know what you are doing, thus not needing to fire the delegate. The cocoa behaviour is also not to notify the delegate when the selection is changed programatically.

Sent from my iPad

On 13 déc. 2011, at 09:22, Randy Lueckereply@reply.github.com wrote:

Id like to know what you all think of this. The cocoa behavior is as we documented. The behavior is a regression. But, I think updating the documentation may be more useful.


Reply to this email directly or view it on GitHub:
#1375 (comment)

Contributor

Me1000 commented Dec 14, 2011

Right, I'm proposing we deviate from cocoa in this case.

Altering the selection programmatically would trigger the willChange: method as well as the did change method.

I really don't know how much more expensive it would be. If you were to change the selection multiple times in the same runloop, maybe... But since you can supply an index set, that would be pretty easily avoidable in 99% of cases.

I'm not entirely sure though. Keeping in compliance with cocoa is probably the best course of action...

I just know when I change the selection programmatically I hate having to fire the notification manually.

Contributor

kuon commented Dec 14, 2011

I took a look at the code that I was writing at the time I reported this issue.

In fact, I'm changing the selection from within tableViewSelectionDidChange, which will fire the notification again, resulting in a loop. I worked around this problem by using a simple mutex boolean variable. But the hardest part was to know what triggered the selection change. In my app, I need different behavior depending on "who" changed the selection. I had to work around that by setting a flag when I'm about to change the selection programmatically, which in the end is much more trouble than firing the notification if needed. (actually the final result is not very complicated, but it took quite some time to get it right, as the behavior was not documented and the source of the selection handling is quite long itself)

Nicolas Goy
Programmer

Goyman.com (http://Goyman.com) SA
Versvey 57
1853 Yvorne
SWITZERLAND

Phone: +41 21 535 31 13
Mobile: +41 79 935 68 93

On Wednesday, 14 December 2011 at 09:40, Randy Luecke wrote:

Right, I'm proposing we deviate from cocoa in this case.

Altering the selection programmatically would trigger the willChange: method as well as the did change method.

I really don't know how much more expensive it would be. If you were to change the selection multiple times in the same runloop, maybe... But since you can supply an index set, that would be pretty easily avoidable in 99% of cases.

I'm not entirely sure though. Keeping in compliance with cocoa is probably the best course of action...

I just know when I change the selection programmatically I hate having to fire the notification manually.


Reply to this email directly or view it on GitHub:
#1375 (comment)

Contributor

aparajita commented Dec 14, 2011

I think it's a mistake to send the notification when you do it programmatically. I think from a design standpoint, the point of being notified that the selection changed is because the user did it, not you. When you change it programmatically, you don't need to be notified, because you are the one doing it. In general I tend to trust Apple's decisions in this regard, I'm sure they thought through the ramifications pretty thoroughly.

It may be that Cocoa doesn't care so much about notifications when changing the selection programmatically because bindings still are updated in that case.

Contributor

Me1000 commented Dec 14, 2011

Yeah, I tend to agree... the situation that @kuon mentioned is certain a valid concern. It's probably best to just fix the regression.

There's about 99% chance the regression involves the binding code, since it's happened before.

cappbot commented May 9, 2012

Milestone: 1.0. Label: #new. What's next? A reviewer should examine this issue.

Contributor

kuon commented May 9, 2012

Anybody has the original text for this issue? It seems to have disappeared. I can't find it in my emails or anywhere. As I wrote a sample code to demonstrate the issue it would help if someone had it.

Owner

aljungberg commented May 14, 2012

Hi. I have added back in what I think was the original description, but I don't think there was any sample code.

Contributor

kuon commented May 14, 2012

@aljungberg Thanks. Hopefully Travis has some sample code that demonstrate the issue.

My original code is still posted, check my comment

@ghost

ghost commented May 21, 2012

#needs-confirmation
+AppKit
+bug

cappbot commented May 21, 2012

Milestone: 1.0. Labels: #needs-confirmation, #new, AppKit, bug. What's next? This issue needs a volunteer to independently reproduce the issue.

Contributor

ahankinson commented Feb 14, 2013

-#new
+#acknowledged

cappbot commented Feb 14, 2013

Milestone: 1.0. Labels: #acknowledged, AppKit, bug. What's next? A reviewer should examine this issue.

Contributor

daboe01 commented Jun 28, 2014

+#needs-patch

cappbot commented Jun 28, 2014

Milestone: 1.0. Labels: #acknowledged, #needs-patch, AppKit, bug. What's next? This issue needs a volunteer to write and submit code to address it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment