From 7d9a52301c59c0b8f3389d06b9d080fbbe714700 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Sat, 14 Mar 2015 14:25:04 -0700 Subject: [PATCH 1/3] Fixed: reloadData in CPTableView run the runLoop Previously, when reloading a CPTableView the run loop was explicitly call to layout the tableView. This is not the case in Cocoa. You can call several times the method reloadData and this will only lay out the tableView one time. --- AppKit/CPTableView.j | 1 - 1 file changed, 1 deletion(-) diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j index bb1db492a..ffe63fc62 100644 --- a/AppKit/CPTableView.j +++ b/AppKit/CPTableView.j @@ -622,7 +622,6 @@ NOT YET IMPLEMENTED - (void)reloadData { [self _reloadDataViews]; - [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; } /*! From 59afb012b6e98df2b166d213ac2638e44a17ad0d Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Sat, 14 Mar 2015 16:18:22 -0700 Subject: [PATCH 2/3] Fixed: make sure to lay out the tableView when calling the method editColumn:row:withEvent:select: --- AppKit/CPTableView.j | 3 +++ 1 file changed, 3 insertions(+) diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j index ffe63fc62..8d664ec38 100644 --- a/AppKit/CPTableView.j +++ b/AppKit/CPTableView.j @@ -5308,6 +5308,9 @@ Your delegate can implement this method to avoid subclassing the tableview to ad [self reloadData]; + // Process all events immediately to make sure table data views are reloaded. + [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; + [self scrollRowToVisible:rowIndex]; [self scrollColumnToVisible:columnIndex]; From 3b2635014cfecc5cbbaa50df82abe7c43143b6a3 Mon Sep 17 00:00:00 2001 From: Alexandre Wilhelm Date: Sun, 15 Mar 2015 19:38:06 -0700 Subject: [PATCH 3/3] Fixed: the developer need to perform the run loop to get some informations in CPTableView. Now, the run loop is performed by the tableview --- AppKit/CPTableView.j | 8 +++ Tests/AppKit/CPTableViewReloadDataTest.j | 5 ++ Tests/AppKit/CPTableViewTableColumnTest.j | 8 +-- Tests/AppKit/CPTableViewTest.j | 62 +++++++++++++++++------ 4 files changed, 64 insertions(+), 19 deletions(-) diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j index 8d664ec38..85bae61ba 100644 --- a/AppKit/CPTableView.j +++ b/AppKit/CPTableView.j @@ -1145,6 +1145,7 @@ NOT YET IMPLEMENTED _dirtyTableColumnRangeIndex = MIN(index, _dirtyTableColumnRangeIndex); [self reloadData]; + [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; } /*! @@ -3833,6 +3834,9 @@ Your delegate can implement this method to avoid subclassing the tableview to ad - (void)enumerateAvailableViewsUsingBlock:(Function/*CPView *dataView, CPInteger row, CPInteger column*, @ref stop*/)handler { [self reloadData]; + + [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; + [self _enumerateViewsInRows:_exposedRows columns:_exposedColumns usingBlock:handler]; } @@ -3842,6 +3846,8 @@ Your delegate can implement this method to avoid subclassing the tableview to ad - (void)_enumerateViewsInRows:(CPIndexSet)rowIndexes columns:(CPIndexSet)columnIndexes usingBlock:(Function/*CPView dataView, CPInteger row, CPInteger column, @ref stop*/)handler { + [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; + [rowIndexes enumerateIndexesUsingBlock:function(rowIndex, stopRow) { var dataViewsForRow = _dataViewsForRows[rowIndex]; @@ -3867,6 +3873,8 @@ Your delegate can implement this method to avoid subclassing the tableview to ad - (void)_enumerateViewsInRows:(CPIndexSet)rowIndexes tableColumns:(CPArray)tableColumns usingBlock:(Function/*CPView dataView, CPInteger row, CPtableColumn tableColumn, CPInteger column, @ref stop*/)handler { + [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; + [rowIndexes enumerateIndexesUsingBlock:function(rowIndex, stopRow) { var dataViewsForRow = _dataViewsForRows[rowIndex]; diff --git a/Tests/AppKit/CPTableViewReloadDataTest.j b/Tests/AppKit/CPTableViewReloadDataTest.j index 5c8a6c63b..fdd99e70a 100644 --- a/Tests/AppKit/CPTableViewReloadDataTest.j +++ b/Tests/AppKit/CPTableViewReloadDataTest.j @@ -41,13 +41,18 @@ [tableView reloadData]; + var enumerateViewsInRowsCall = 0; + [tableView _enumerateViewsInRows:[CPIndexSet indexSetWithIndexesInRange:CPMakeRange(0, 3)] columns:[CPIndexSet indexSetWithIndexesInRange:CPMakeRange(0, 4)] usingBlock:function(view, row, column, stop) { var tableColumn = [[tableView tableColumns] objectAtIndex:column], expected = [tableColumn identifier] + "_" + [tableContent objectAtIndex:row]; [self assertTrue:([view stringValue] == expected)]; + enumerateViewsInRowsCall++; }]; + + [self assert:enumerateViewsInRowsCall equals:12]; } - (int)numberOfRowsInTableView:(CPTableView)aTableView diff --git a/Tests/AppKit/CPTableViewTableColumnTest.j b/Tests/AppKit/CPTableViewTableColumnTest.j index a63381e73..261e4a734 100644 --- a/Tests/AppKit/CPTableViewTableColumnTest.j +++ b/Tests/AppKit/CPTableViewTableColumnTest.j @@ -40,19 +40,21 @@ [self assertTrue:([[tableView tableColumns] count] == 4)]; [tableView removeTableColumn:[[tableView tableColumns] firstObject]]; -[[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; [self assertTrue:([[tableView tableColumns] count] == 3)]; [tableView removeTableColumn:[[tableView tableColumns] firstObject]]; -[[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; [self assertTrue:([[tableView tableColumns] count] == 2)]; + var enumerateViewsInRowsCall = 0; + [tableView enumerateAvailableViewsUsingBlock:function(view, row, column, stop) { var tableColumn = [[tableView tableColumns] objectAtIndex:column]; [self assert:("COLUMN_" + [tableColumn identifier] + "ROW_" + row) equals:[view objectValue]]; - + enumerateViewsInRowsCall++; }]; + + [self assert:enumerateViewsInRowsCall equals:30]; } - (int)numberOfRowsInTableView:(CPTableView)aTableView diff --git a/Tests/AppKit/CPTableViewTest.j b/Tests/AppKit/CPTableViewTest.j index 24add388d..e3e29dccb 100644 --- a/Tests/AppKit/CPTableViewTest.j +++ b/Tests/AppKit/CPTableViewTest.j @@ -159,9 +159,6 @@ [tableView selectRowIndexes:[CPIndexSet indexSetWithIndex:1] byExtendingSelection:NO]; [tableView editColumn:0 row:1 withEvent:nil select:YES]; - // Process all events immediately to make sure table data views are reloaded. - [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; - // Now some text field should be the first responder. var fieldEditor = [theWindow firstResponder]; [self assert:CPTextField equals:[fieldEditor class] message:"table cell editor should be a text field"]; @@ -184,6 +181,7 @@ { var scrollView = [[CPScrollView alloc] initWithFrame:CGRectMake(0, 0, 100.0, 100.0)], tableColumn1 = [[CPTableColumn alloc] initWithIdentifier:@"Bar"]; + [tableView addTableColumn:tableColumn1]; [scrollView setDocumentView:tableView]; @@ -206,6 +204,8 @@ [tableColumn setDataView:[CustomTextView0 new]]; [tableColumn1 setDataView:[CustomTextView1 new]]; + + [tableView reloadData]; // Process all events immediately to make sure table data views are reloaded. [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; @@ -238,13 +238,15 @@ visibleHeight = [tableView visibleRect].size.height, fullRowHeight = [tableView rowHeight] + [tableView intercellSpacing].height, visibleRows = CEIL(visibleHeight / fullRowHeight); + [self assert:2 * visibleRows equals:[allViews count] message:"only as many data views as necessary should be present"]; // Now if we scroll down, new views should come in and others should go out. var rowTwentyFiveAndAHalfY = FLOOR(25.5 * fullRowHeight); [tableView scrollPoint:CGPointMake(0, rowTwentyFiveAndAHalfY)]; - [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; + + [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; AssertCorrectCellsVisible(25); [self assert:2 * visibleRows equals:[allViews count] message:"only as many data views as necessary should be present (2)"]; } @@ -270,11 +272,11 @@ [[theWindow contentView] addSubview:contentBindingTable]; [theWindow makeFirstResponder:contentBindingTable]; - [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; - // Set the model again with different values [delegate setTableEntries:[[@"C1", @"D1"], [@"C2", @"D2"], [@"C3", @"D3"]]]; [contentBindingTable reloadData]; + + [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; } - (void)testColumnValueBinding @@ -294,8 +296,6 @@ [[theWindow contentView] addSubview:table]; [theWindow makeFirstResponder:table]; - [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; - // Should remove all table rows. [self assertNoThrow:function() { @@ -314,13 +314,18 @@ // Change a value in row 0. The number of rows stays the same. [container setTableEntries:@[@{@"name":@"B4"}, @{@"name":@"B2"}, @{@"name":@"B3"}]]; + var enumerateViewsInRowsCall = 0; + // Checks that the displayed data matches the model data. [table enumerateAvailableViewsUsingBlock:function(dataView, aRow, aColumn, stop) { + enumerateViewsInRowsCall++; var data_value = [[[container tableEntries] objectAtIndex:aRow] objectForKey:@"name"]; [self assert:[dataView objectValue] equals:data_value]; }]; + [self assert:enumerateViewsInRowsCall equals:3]; + // We could also check that the dataviews in rows 1 and 2 are identical to the ones before mutation. } @@ -349,8 +354,6 @@ [table reloadData]; - [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; - [self assertTrue:[table bounds].size.width > 100 && [table bounds].size.width < 200]; [tableColumn1 setHidden:NO]; @@ -429,9 +432,13 @@ [self assert:CPNotFound equals:column]; [self assert:CPNotFound equals:row]; + var enumerateViewsInRowsCall = 0; + // Enumerate views inside the table view and check that rows and columns are correct [table enumerateAvailableViewsUsingBlock:function(dataView, aRow, aColumn, stop) { + enumerateViewsInRowsCall++; + var getRow, getColumn; @@ -441,9 +448,15 @@ [self assert:aRow equals:getRow]; }]; + [self assert:enumerateViewsInRowsCall equals:6]; + + enumerateViewsInRowsCall = 0; + // Enumerate views inside a different table view and check that rows and columns are not found [table2 enumerateAvailableViewsUsingBlock:function(dataView, aRow, aColumn, stop) { + enumerateViewsInRowsCall++; + var getRow, getColumn; @@ -452,6 +465,8 @@ [self assert:CPNotFound equals:getRow]; [self assert:CPNotFound equals:getColumn]; }]; + + [self assert:enumerateViewsInRowsCall equals:6]; } -(void)testNotificationsRegistered @@ -498,6 +513,8 @@ [theWindow makeFirstResponder:tableView]; [tableView selectRowIndexes:[CPIndexSet indexSetWithIndex:0] byExtendingSelection:NO]; + var enumerateViewsInRowsCall = 0; + [tableView enumerateAvailableViewsUsingBlock:function(dataView, row, column, stop) { if (row == 0) @@ -513,12 +530,20 @@ [self assertFalse:[dataView hasThemeState:CPThemeStateSelectedDataView] message:"CPThemeStateSelectedDataView should be disabled"]; [self assertTrue:[dataView hasThemeState:CPThemeStateFirstResponder] message:"CPThemeStateFirstResponder should be enabled"]; } + + enumerateViewsInRowsCall++; }]; + [self assert:enumerateViewsInRowsCall equals:3]; + [tableView selectRowIndexes:[CPIndexSet indexSetWithIndex:1] byExtendingSelection:NO]; + enumerateViewsInRowsCall = 0; + [tableView enumerateAvailableViewsUsingBlock:function(dataView, row, column, stop) { + enumerateViewsInRowsCall++; + if (row == 0) { [self assertTrue:[dataView hasThemeState:CPThemeStateTableDataView] message:"CPThemeStateTableDataView should be enabled"]; @@ -527,10 +552,16 @@ } }]; + [self assert:enumerateViewsInRowsCall equals:3]; + [theWindow makeFirstResponder:textField]; + enumerateViewsInRowsCall = 0; + [tableView enumerateAvailableViewsUsingBlock:function(dataView, row, column, stop) { + enumerateViewsInRowsCall++; + if (row == 0) { [self assertTrue:[dataView hasThemeState:CPThemeStateTableDataView] message:"CPThemeStateTableDataView should be enabled"]; @@ -545,6 +576,8 @@ [self assertFalse:[dataView hasThemeState:CPThemeStateFirstResponder] message:"CPThemeStateFirstResponder should be disabled"]; } }]; + + [self assert:enumerateViewsInRowsCall equals:3]; } - (void)testMethodViewAtColumnWithMakeIfNecessarySetToNo @@ -558,6 +591,9 @@ [dataSource setTableEntries:["A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"]]; [tableView setDataSource:dataSource]; + var view = [tableView viewAtColumn:0 row:0 makeIfNecessary:NO]; + [self assert:view equals:nil message:@"View should be equal to nil"]; + // Process all events immediately to make sure table data views are reloaded. [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; @@ -580,9 +616,6 @@ [dataSource setTableEntries:["A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"]]; [tableView setDataSource:dataSource]; - // Process all events immediately to make sure table data views are reloaded. - [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; - var view = [tableView viewAtColumn:0 row:0 makeIfNecessary:YES]; [self assert:[view objectValue] equals:@"A" message:@"View should be equal to A"]; [self assert:[view superview] equals:tableView message:@"Superview of view should be the tableview"]; @@ -603,9 +636,6 @@ [dataSource setTableEntries:["A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"]]; [tableView setDataSource:dataSource]; - // Process all events immediately to make sure table data views are reloaded. - [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode]; - var expectedMessage = @"Row 26 out of row range [0-25] for rowViewAtRow:createIfNeeded:", exceptionMessage = @"";