From 6bc109f7fc8141224b67cf65db65a41609f5aabc Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Thu, 13 Jan 2011 20:51:43 -0300 Subject: [PATCH] Optimised: outline view expandItem:expandChildren:YES sent repeated selection change notifications. --- AppKit/CPOutlineView.j | 99 ++++++++++++++++++-------------- Tests/AppKit/CPOutlineViewTest.j | 14 ++++- 2 files changed, 70 insertions(+), 43 deletions(-) diff --git a/AppKit/CPOutlineView.j b/AppKit/CPOutlineView.j index b3fc0db84..6a4a76195 100644 --- a/AppKit/CPOutlineView.j +++ b/AppKit/CPOutlineView.j @@ -75,6 +75,10 @@ var CPOutlineViewDelegate_outlineView_dataViewForTableColumn_item_ CPOutlineViewDropOnItemIndex = -1; +var CPOutlineViewCoalesceSelectionNotificationStateOff = 0, + CPOutlineViewCoalesceSelectionNotificationStateOn = 1, + CPOutlineViewCoalesceSelectionNotificationStateDid = 2; + /*! @ingroup appkit @class CPOutlineView @@ -114,7 +118,7 @@ CPOutlineViewDropOnItemIndex = -1; CPTimer _dragHoverTimer; id _dropItem; - BOOL _suppressSelectionNotifications; + BOOL _coalesceSelectionNotificationState; } - (id)initWithFrame:(CGRect)aFrame @@ -123,7 +127,6 @@ CPOutlineViewDropOnItemIndex = -1; if (self) { - _selectionHighlightStyle = CPTableViewSelectionHighlightStyleSourceList; // The root item has weight "0", thus represents the weight solely of its descendants. @@ -329,14 +332,23 @@ CPOutlineViewDropOnItemIndex = -1; if (!itemInfo) return; + // When shouldExpandChildren is YES, we need to make sure we're collecting + // selection notifications so that exactly one IsChanging and one DidChange + // is sent as needed, for the totallity of the operation. + var isTopLevel = NO; + if (!_coalesceSelectionNotificationState) + { + isTopLevel = YES; + _coalesceSelectionNotificationState = CPOutlineViewCoalesceSelectionNotificationStateOn; + } + // to prevent items which are already expanded from firing notifications if (!itemInfo.isExpanded) { [self _noteItemWillExpand:anItem]; // Shift selection indexes below so that the same items remain selected. - var newRowCount = [_outlineViewDataSource outlineView:self numberOfChildrenOfItem:anItem], - newSelection = nil; + var newRowCount = [_outlineViewDataSource outlineView:self numberOfChildrenOfItem:anItem]; if (newRowCount) { var selection = [self selectedRowIndexes], @@ -346,7 +358,7 @@ CPOutlineViewDropOnItemIndex = -1; { [self _noteSelectionIsChanging]; [selection shiftIndexesStartingAtIndex:expandIndex by:newRowCount]; - newSelection = selection; + [self _setSelectedRowIndexes:selection]; // _noteSelectionDidChange will be suppressed. } } @@ -354,12 +366,6 @@ CPOutlineViewDropOnItemIndex = -1; // XXX Shouldn't the items reload before the notification is sent? [self _noteItemDidExpand:anItem]; [self reloadItem:anItem reloadChildren:YES]; - - // Update the selection - and send the associated notification - first - // after the items have loaded so that the new selection is consistent - // with the actual rows for any observers. - if (newSelection) - [self _setSelectedRowIndexes:newSelection]; // Will call _noteSelectionDidChange } if (shouldExpandChildren) @@ -370,6 +376,14 @@ CPOutlineViewDropOnItemIndex = -1; while (childIndex--) [self expandItem:children[childIndex] expandChildren:YES]; } + + if (isTopLevel) + { + var r = _coalesceSelectionNotificationState; + _coalesceSelectionNotificationState = CPOutlineViewCoalesceSelectionNotificationStateOff; + if (r === CPOutlineViewCoalesceSelectionNotificationStateDid) + [self _noteSelectionDidChange]; + } } /*! @@ -390,6 +404,9 @@ CPOutlineViewDropOnItemIndex = -1; if (!itemInfo.isExpanded) return; + // Don't spam notifications. + _coalesceSelectionNotificationState = CPOutlineViewCoalesceSelectionNotificationStateOn; + [self _noteItemWillCollapse:anItem]; // Update selections: // * Deselect items inside the collapsed item. @@ -401,8 +418,7 @@ CPOutlineViewDropOnItemIndex = -1; while (collapseEndIndex + 1 < _itemsForRows.length && [self levelForRow:collapseEndIndex + 1] > topLevel) collapseEndIndex++; - var collapseRange = CPMakeRange(collapseTopIndex + 1, collapseEndIndex - collapseTopIndex), - newSelection = nil; + var collapseRange = CPMakeRange(collapseTopIndex + 1, collapseEndIndex - collapseTopIndex); if (collapseRange.length) { @@ -410,38 +426,31 @@ CPOutlineViewDropOnItemIndex = -1; if ([selection intersectsIndexesInRange:collapseRange]) { - [selection removeIndexesInRange:collapseRange]; [self _noteSelectionIsChanging]; - newSelection = selection; + [selection removeIndexesInRange:collapseRange]; + [self _setSelectedRowIndexes:selection]; // _noteSelectionDidChange will be suppressed. } // Shift any selected rows below upwards. if ([selection intersectsIndexesInRange:CPMakeRange(collapseEndIndex + 1, _itemsForRows.length)]) { - // Notify if that wasn't already done above. - if (!newSelection) - [self _noteSelectionIsChanging]; - + [self _noteSelectionIsChanging]; [selection shiftIndexesStartingAtIndex:collapseEndIndex + 1 by:-collapseRange.length]; - newSelection = selection; + [self _setSelectedRowIndexes:selection]; // _noteSelectionDidChange will be suppressed. } } itemInfo.isExpanded = NO; // XXX Shouldn't the items reload before the notification is sent? [self _noteItemDidCollapse:anItem]; - - // Reload item calls [super reload], which can lead to hanging selection - // removal and a selection notification before we are consistent. - _suppressSelectionNotifications = YES; [self reloadItem:anItem reloadChildren:YES]; - _suppressSelectionNotifications = NO; - // Update the selection - and send the associated notification - first - // after the items have loaded so that the new selection is consistent - // with the actual rows for any observers. - if (newSelection) - [self _setSelectedRowIndexes:newSelection]; // Will call _noteSelectionDidChange + // Send selection notifications only after the items have loaded so that + // the new selection is consistent with the actual rows for any observers. + var r = _coalesceSelectionNotificationState; + _coalesceSelectionNotificationState = CPOutlineViewCoalesceSelectionNotificationStateOff; + if (r === CPOutlineViewCoalesceSelectionNotificationStateDid) + [self _noteSelectionDidChange]; } /*! @@ -1294,13 +1303,16 @@ CPOutlineViewDropOnItemIndex = -1; */ - (void)_noteSelectionIsChanging { - if (_suppressSelectionNotifications) - return; + if (!_coalesceSelectionNotificationState || _coalesceSelectionNotificationState === CPOutlineViewCoalesceSelectionNotificationStateOn) + { + [[CPNotificationCenter defaultCenter] + postNotificationName:CPOutlineViewSelectionIsChangingNotification + object:self + userInfo:nil]; + } - [[CPNotificationCenter defaultCenter] - postNotificationName:CPOutlineViewSelectionIsChangingNotification - object:self - userInfo:nil]; + if (_coalesceSelectionNotificationState === CPOutlineViewCoalesceSelectionNotificationStateOn) + _coalesceSelectionNotificationState = CPOutlineViewCoalesceSelectionNotificationStateDid; } /*! @@ -1308,13 +1320,16 @@ CPOutlineViewDropOnItemIndex = -1; */ - (void)_noteSelectionDidChange { - if (_suppressSelectionNotifications) - return; + if (!_coalesceSelectionNotificationState) + { + [[CPNotificationCenter defaultCenter] + postNotificationName:CPOutlineViewSelectionDidChangeNotification + object:self + userInfo:nil]; + } - [[CPNotificationCenter defaultCenter] - postNotificationName:CPOutlineViewSelectionDidChangeNotification - object:self - userInfo:nil]; + if (_coalesceSelectionNotificationState === CPOutlineViewCoalesceSelectionNotificationStateOn) + _coalesceSelectionNotificationState = CPOutlineViewCoalesceSelectionNotificationStateDid; } /*! diff --git a/Tests/AppKit/CPOutlineViewTest.j b/Tests/AppKit/CPOutlineViewTest.j index 24020d3fa..5208e833d 100644 --- a/Tests/AppKit/CPOutlineViewTest.j +++ b/Tests/AppKit/CPOutlineViewTest.j @@ -105,6 +105,7 @@ afterSelection = [outlineView selectedRowIndexes]; [self assert:1 equals:[afterSelection count] message:"1 selection should disappear"]; [self assert:".3" equals:[outlineView itemAtRow:[afterSelection firstIndex]] message:".3 selection should remain"]; + [self assert:1 equals:[delegate selectionChangeCount] message:"selection notifications during collapseItem"]; } /*! @@ -128,12 +129,13 @@ [outlineView setDelegate:delegate]; [outlineView expandItem:".1.2"]; - afterSelection = [outlineView selectedRowIndexes]; + afterSelection = [outlineView selectedRowIndexes]; [self assert:2 equals:[afterSelection count] message:"selections should remain"]; [self assert:".1.1" equals:[outlineView itemAtRow:[afterSelection firstIndex]] message:".1.1 selection should remain"]; [self assert:".3.1" equals:[outlineView itemAtRow:[afterSelection lastIndex]] message:".3.1 selection should remain"]; + [self assert:1 equals:[delegate selectionChangeCount] message:"selection notifications during expandItem"]; } @end @@ -183,10 +185,20 @@ { id tester @accessors; CPArray expectedSelectedItems @accessors; + int selectionChangeCount @accessors; +} + +- (id)init +{ + if (self = [super init]) + selectionChangeCount = 0; + return self; } - (void)outlineViewSelectionDidChange:(CPNotification)aNotification { + selectionChangeCount++; + // Verify that the state is consistent - every selected row has been loaded. var anOutlineView = [aNotification object], selection = [anOutlineView selectedRowIndexes],