controlsfx / controlsfx

High quality UI controls to complement the core JavaFX distribution
https://controlsfx.org
BSD 3-Clause "New" or "Revised" License
1.58k stars 269 forks source link

Filter for SpreadsheetView #901

Closed JonathanGiles closed 7 years ago

JonathanGiles commented 7 years ago

Original report by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Hi guys,

I'm creating this issue to continue the discussion about filters in the SpreadsheetView (initially here : https://groups.google.com/forum/#!topic/controlsfx-dev/6mGkogDQXVY)

My goal is to be able to apply filters in the SpreadsheetView just like Excel in the same manner that @thomasnield did it for the TableView.

What I want :

I want to do what Excel is doing (maybe a bit less). That is to say be able to select a cell and make it the Filter cell. This cell will have a little arrow on it, allowing to filter all cells situated below it. When I click on the little arrow, I will have several choices, but essentially specialized filters (textual, numeric etc) and a Table showing me all values like in TableFilter. filter1.png

Where is the difficulty?

The SpreadsheetView provides a large range of features that allow it to do far more things that a TableView can :

Because of these features, some basic thing are disabled in the SpreadsheetView:

This is due to the main difference between the TableView and the SpreadsheetView. TableView is designed to show one of several column headers, with the same type of data in it. Basically show some lines extracted from a Relational database. The SpreadsheetView is designed to create Spreadsheet where you will have an Image at the beginning, then some headers, some data, and an Image again and another headers. Basically something like that (this is a SpreadsheetView) : GKWviBw.png

So implementation of the filters will be difficult because :

What are you going to do :

This feature is so huge and so complex that I initially thought doing it on my side. This allow me to tweak the SpreadsheetView like I want, not bother of the API and some other reasons.

But I know some people would want a feature like that. So the idea is to work step by step. Providing some basic layers. Then going forward.

For example, the first step is to implement an API that will allow a cell to be a filter for the column. Once a filter is set, the SpreadsheetView will provide a button allowing to show a Popup. No more. It's already complex :

This simple thing will allow people (including me) to implement their own Popup and do what they want.

Then we can think about going forward. Provide basic filters to the SpreadsheetView. Allow the SpreadsheetView to remove and add some rows. This is a very difficult thing (see https://bitbucket.org/controlsfx/controlsfx/pull-requests/564/row-adding-and-removing-capabilities/diff ). Etc etc.

I will begin to work on that, start the modification on the SpreadsheetView. Any help, ideas, snippets, thoughts is very welcomed.

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Resolved by pull request #627

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Pull request #627 is submitted. Anyone can review the code, fork the pull request and play with it!

JonathanGiles commented 7 years ago

Original comment by Thomas Nield (Bitbucket: thomasnield, GitHub: thomasnield).


Nice work Samir!

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Pull request will be available as soon as possible! Proud to add filters AND sort in the SpreadsheetView! Capture.PNG

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Since I'm working on showing and hiding rows, I'll also add the possibility to hide and show columns. I though this would be difficult but I forgot it was already implemented in the TableView! Thus I hope it will be easier.

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Allright, my work can be visible on the branch SpreadsheetViewFilters in my repo : https://bitbucket.org/shadzic/controlsfx/branch/SpreadsheetViewFilters

Basically, I want this to be as simple as possible for the end user. I initially put the FIlteredList on the Grid (model) side but I think it's wrong. The model should stay the model and have the complete list of rows all the time. The SpreadsheetView should have the FilteredList internally and handle it itself.

I will integrate this in my application. The rest of the job is to go a bit everywhere in the code, and chase where the index are used in order to use the right ones. When inside the SpreadsheetView, the view index should be used. When dealing with the Grid and the outside, the model index should be used..

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Yes I will get into TableFilter tomorrow to understand how you've done it. Now that I know (not 100%...) it can actually work, I can focus on performance and the actual implementation.

JonathanGiles commented 7 years ago

Original comment by Thomas Nield (Bitbucket: thomasnield, GitHub: thomasnield).


Cool! If you want study my TableView, that might be helpful. I learned a lot in hindsight and rebuilt its implementation twice, and ultimately discovered some strategies to overcome some severe performance issues. I think I used a HashMap with a DupeCounter if I recall. If I get time I'll look at it later.

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Here are some methods added in the SpreadsheetView :

#!java

 //The visible rows.
    private BitSet hiddenRows;
    private HashMap<Integer,Integer> rowMap = new HashMap<>();
    private HashMap<Integer,Integer> reverseRowMap = new HashMap<>();
    private Integer filteredRow;
    public boolean isRowHidden(int row){
        return hiddenRows.get(row);
    }

    public BitSet getHiddenRows(){
        return hiddenRows;
    }

    public void setHiddenRows(BitSet hiddenRows){
        this.hiddenRows = hiddenRows;
        computeRowMap();
        requestLayout();
    }

    public Integer getFilteredRow(){
        return filteredRow;
    }

    public void setFilteredRow(Integer row) {
        if (row == null || row > getGrid().getRowCount()) {
            filteredRow = null;
        } else {
            filteredRow = row;
        }
    }

    public void hideRow(int row){
        if(hiddenRows.get(row)){
            return;
        }
        hiddenRows.set(row, false);
        computeRowMap();
    }

    private void computeRowMap(){
        rowMap = new HashMap<>(getGrid().getRows().getSource().size());
        reverseRowMap = new HashMap<>(getGrid().getRows().getSource().size());
        int visibleRow = 0;
        for(int i=0;i<getGrid().getRows().getSource().size();++i ){
            reverseRowMap.put(visibleRow, i);
            if(!hiddenRows.get(i)){
               rowMap.put(i,visibleRow++);
            }else{
                rowMap.put(i,visibleRow);
            }
        }
        getGrid().getRows().setPredicate(new Predicate<ObservableList<SpreadsheetCell>>() {
            @Override
            public boolean test(ObservableList<SpreadsheetCell> t) {
                return !hiddenRows.get(getGrid().getRows().getSource().indexOf(t));
            }
        });
    }

    public void showRow(int row){
        if(!hiddenRows.get(row)){
            return;
        }
         hiddenRows.set(row, true);
        computeRowMap();
    }

    public int getViewRow(int modelRow){
        try{
        return rowMap.get(modelRow);
        }catch(NullPointerException ex){
            return modelRow;
        }
    }

    public int getModelRow(int viewRow){
         try{
        return reverseRowMap.get(viewRow);
         }catch(NullPointerException ex){
            return viewRow;
        }
    }

    public int getRowSpan(SpreadsheetCell cell) {
        int rowSpan = cell.getRowSpan();
        for (int i = cell.getRow(); i < cell.getRow() + cell.getRowSpan(); ++i) {
            rowSpan -= hiddenRows.get(i) ? 1 : 0;
        }
        return rowSpan;
    }
JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Allright I've made a simple test by making the ObservableList of all rows a FilteredList, then applying a Predicate that will hide all hidden rows.

I initially thought about modify the SpreadsheetCell row and rowSpan, but it's the model so I cannot allow myself to do that. Instead I maintain two HashMap<Integer,Integer> that are doing the link between the row in a SpreadsheetCell (model row) and the actual row in the SpreadsheetView. Therefore the getRow and getRowSpan of a cell is always accessed through the SpreadsheetView. Here is quick demo of what can be done: https://samirhadzic.com/images/filterSpv.webm

I have not analyzed yet the impact on performance, I need to first see if it's working, then optimize it and I'll measure it. But I think it should not be that much although I'm unsure about the inner mechanism of FilteredList.. On one side we could have two distinct grids and on the other two HashMap<int,int>.

https://samirhadzic.com/images/filterSpv.webm

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Yes each cell is contained into an ObservableList, and each row (ObservableList) is also contained into an ObservableList.

Each SpreadsheetCell has its row index, I could use indexOf applied to the row but this would be not suitable since complexity of this method is O(n). EDIT: My only problem is to quickly be able to get the correct index of a Cell to determine in which row it is. When I'm in the view (TableCell etc), no problem. But when I'm dealing with the model, I don't see a quick way to determine it.

JonathanGiles commented 7 years ago

Original comment by Thomas Nield (Bitbucket: thomasnield, GitHub: thomasnield).


The TableFilter will commandeer the backing items ObservableList and replace it with a FilteredList it controls. I didn't look at the implementation of this, but is there a notion of each row being contained in an ObservableList or is it much more abstract than that?

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


I've been working on this feature and made some progress. I've been able to put some Filter on a row and several columns : Capture.PNG

Now that this is done, I want to go forward and add the ability to hide/show some rows. I initially thought about duplicating the grids but that cannot be a solution. So I'm thinking about removing the hidden rows inside the Grid model, and storing them somewhere else. I need to update the span of the SpreadsheetCell but I cannot (But I'm unsure) update the SpreadsheetCell row because I'm not sure I'll be able to get on my feet when showing the row again. Also the model should mirror the reality, if a cell is on row 5, it should always be on row 5 no matter if row 2,3 and 4 have been hidden.

This leads us to the way of managing these wrong row indexes within the SpreadsheetView. I do not see any other solution than keeping a Map doing the link between the row in my Grid (SpreadsheetCell) and the rows inside the SpreadsheetView (visible rows). Therefore getRow() method of the SpreadsheetCell should not be used, and always be accessed through the Map.. I'm quite afraid of the consequences because this thing will spread through all the code but it could a wonderful feature for the SpreadsheetView.

Any though @JonathanGiles @thomasnield ?

JonathanGiles commented 7 years ago

Original comment by Thomas Nield (Bitbucket: thomasnield, GitHub: thomasnield).


No good reason. I should have. I can move it later of you like or you are welcome to.

JonathanGiles commented 7 years ago

Original comment by Samir Hadzic (Bitbucket: shadzic, GitHub: shadzic).


Just one question aside @thomasnield , why didn't you extends ControlsFXSample in FlightTable and others application into ControlsFXSamples? because your controls are not visible when we are launching the sampler..

JonathanGiles commented 7 years ago

Original comment by Thomas Nield (Bitbucket: thomasnield, GitHub: thomasnield).


If we need to extract a base abstract implementation of the filter control let me know. I recall Jonathon Giles wanted to make it available for TreeTableView awhile ago but I haven't heard any requests for it since.