From 6e2dcd5dcbde7e5eba5125a2772304cc87f6b64a Mon Sep 17 00:00:00 2001 From: Andrew Hankinson Date: Mon, 15 Apr 2013 11:46:58 -0400 Subject: [PATCH 1/3] Fixed: Delegate method fires twice when using keyboard to select rows in table view Previously, navigating through the rows of a tableview with the arrow keys would cause the delegate method `- (BOOL)tableView:(CPTableView)aTableView shouldSelectRow:(int)rowIndex` to fire twice. This was due to the delegate method being called twice in checking whether the row could be selected. This commit changes it so that the delegate method is called just once and the result stored. --- AppKit/CPTableView.j | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j index 01f5e6765..5ae5d721b 100644 --- a/AppKit/CPTableView.j +++ b/AppKit/CPTableView.j @@ -5235,12 +5235,13 @@ Your delegate can implement this method to avoid subclassing the tableview to ad if (_implementedDelegateMethods & CPTableViewDelegate_tableView_shouldSelectRow_) { + var shouldSelect = [_delegate tableView:self shouldSelectRow:i]; - while (![_delegate tableView:self shouldSelectRow:i] && (i < [self numberOfRows] && i > 0)) + while (!shouldSelect && (i < [self numberOfRows] && i > 0)) shouldGoUpward ? i-- : i++; //check to see if the row can be selected if it can't be then see if the next row can be selected // If the index still can be selected after the loop then just return. - if (![_delegate tableView:self shouldSelectRow:i]) + if (!shouldSelect) return; } From 3612565e3a7eff35e82154736bdc3c84d47c3c83 Mon Sep 17 00:00:00 2001 From: Andrew Hankinson Date: Mon, 15 Apr 2013 16:38:37 -0400 Subject: [PATCH 2/3] Fix delegate selection to check the index first In the previous patch the shouldSelectRow delegate was being stored and only called once. This changes its behaviour again to first fire the delegate to see if the row in question can be selected. If not, it will look for the next row (up or down) that may be selected and choose that one. --- AppKit/CPTableView.j | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j index 5ae5d721b..67f837d51 100644 --- a/AppKit/CPTableView.j +++ b/AppKit/CPTableView.j @@ -5236,13 +5236,18 @@ Your delegate can implement this method to avoid subclassing the tableview to ad if (_implementedDelegateMethods & CPTableViewDelegate_tableView_shouldSelectRow_) { var shouldSelect = [_delegate tableView:self shouldSelectRow:i]; - - while (!shouldSelect && (i < [self numberOfRows] && i > 0)) - shouldGoUpward ? i-- : i++; //check to see if the row can be selected if it can't be then see if the next row can be selected - - // If the index still can be selected after the loop then just return. if (!shouldSelect) - return; + { + /* If shouldSelect returns NO it means this row cannot be selected. + The proper behaviour is to then try to see if the next/previous + row(s) can be selected, until we hit the first one that can be. + */ + while (!shouldSelect && (i < [self numberOfRows] && i > 0)) + { + shouldGoUpward ? i-- : i++; //check to see if the row can be selected if it can't be then see if the next row can be selected + shouldSelect = [_delegate tableView:self shouldSelectRow:i]; + } + } } // If we go upward and see that this row is already selected we should deselect the row below. From 2974bdc1d76d9b46091f0ce5a06b3bbca74ff56a Mon Sep 17 00:00:00 2001 From: Andrew Hankinson Date: Fri, 19 Apr 2013 16:45:40 -0400 Subject: [PATCH 3/3] Fix formatting and enumerator problems --- AppKit/CPTableView.j | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j index 67f837d51..1d5cba52d 100644 --- a/AppKit/CPTableView.j +++ b/AppKit/CPTableView.j @@ -5236,6 +5236,7 @@ Your delegate can implement this method to avoid subclassing the tableview to ad if (_implementedDelegateMethods & CPTableViewDelegate_tableView_shouldSelectRow_) { var shouldSelect = [_delegate tableView:self shouldSelectRow:i]; + if (!shouldSelect) { /* If shouldSelect returns NO it means this row cannot be selected. @@ -5244,10 +5245,11 @@ Your delegate can implement this method to avoid subclassing the tableview to ad */ while (!shouldSelect && (i < [self numberOfRows] && i > 0)) { - shouldGoUpward ? i-- : i++; //check to see if the row can be selected if it can't be then see if the next row can be selected + shouldGoUpward ? --i : ++i; //check to see if the row can be selected if it can't be then see if the next row can be selected shouldSelect = [_delegate tableView:self shouldSelectRow:i]; } } + } // If we go upward and see that this row is already selected we should deselect the row below.