evrencoskun / TableView

TableView is a powerful Android library for displaying complex data structures and rendering tabular data composed of rows, columns and cells.
MIT License
3.13k stars 453 forks source link

Refactor the corner view visibility #375

Closed MGaetan89 closed 3 years ago

MGaetan89 commented 3 years ago

I've refactored the AbstractTableAdapter#setAllItems method and added some tests for the corner view logic (creation and visibility).

This change fixes an issue where the corner view could remain visible when toggling TableView#mShowCornerView from true to false.

Zardozz commented 3 years ago

There is two problems with this pull request.

1) AbstractTableAdapterTest class is really in the wrong directory, it should be located under the "test" directory (have a package name of "com.evrencoskun.tableview.test"), this might not currently affect the tests but it is not the correct structure for this type of test.

The reason it should be under the "test" directory and package name of "com.evrencoskun.tableview.test" is that androidTest auto generates the androidTest resources under the "com.evrencoskun.tableview.test" thus making it hard to use them if not in the right package (though this class does not use them at the moment so it works).

2) The change breaks the fix for #308 for which you fixed by altering the tests by inverting there assertions in the CornerViewTest

setShowCornerView method is to set a Flag to change the behaviour of setAllItems without breaking the current behaviour

testColumnHeadersOnlyTableShowCorner and testColumnHeadersOnlyTableShowCornerResetEmptyTable and testColumnHeadersOnlyTableShowCornerResetNonEmptyTable Tests
set the cornerView to be shown even if they have no row data or row headers but do have column header data.

These tests in CornerViewTest need to pass without any changes to the tests.

The idea of the tests

// Set the option to show corner view when there is not row data
        tableView.setShowCornerView(true);

is the cornerView is created (i.e. Not Null) Assert.assertNotNull(cornerViewReset);

even when there are no rows

// Only want column headers
        SimpleData simpleData = new SimpleData(5, 0);

basically if setShowCornerView is true then the test in your code of boolean hasRowHeaders = rowHeaderItems != null && !rowHeaderItems.isEmpty(); should be ignored (or always return true)

MGaetan89 commented 3 years ago

Thanks for having a look at my PR. Indeed my approach was originally wrong. I've updated my PR, so now the original tests remain unchanged, and it still fixes a couple of corner view visibility.

MGaetan89 commented 3 years ago

@evrencoskun any chance to have a look at this?