My current solution for this bug is that i use a custom EditCellCommandHandler which simply re-sets the active cell editor so that the table does not know that there is one. But this has certain drawbacks as i need to worry about closing the editor if the user clicks somewhere.
Why is the editor not stored as an instance variable inside of the table? Every call agains the static helper EditUtils.commitAndCloseActiveEditor() has either access to the natTable itself or has an event that holds the nat table. This would be a quick fix and would not require any major refactoring as stated in the bug report.
I am thinking about patching the NatTable so i do not need all those workarounds ...
I am done with my changes Everything works as expected and I could get rid of those static references to the cell editor. The only remaining "problem" is that I do not know which repository you are using for external contributions.
PS: Thanks for the great tutorial how to setup the workspace. Using that I could get started in a few minutes
Cool news. There is no repository for external contributions. You just need to attach a patch containing your modifications to the ticket. And you need to follow the Eclipse development process, so the patch needs to be signed, you need to hace the CLA signed,...
Gerrit would be cool, makes the life as contributor a little bit easier.
I agree, just need to check what I need to do to enable Gerrit for NatTable.
I did a quick check by viewing the patch, and the first impression is positive. I will apply the patch and perform various tests the next days. There are so much corner cases that are difficult to deal with.
The solution for ScrollBarHandlerTemplate is really ugly, but not sure if there is another way. I will see if there is another option for that.
Thanks for the contribution so far, I will get back after the checks are finished.
Michael Heiß Messages: 19 Registered: November 2013 Location: Graz, Austria
I found one problem in the patch that I proposed. The CellEditorCreatedEvent#convertToLocal should always return true. I honestly don't know why the instance of check is in. Will update the patch tomorrow.
Regarding the ScrollBarHandlerTemplate: What if I introduce another LayerEvent (CellEditorCloseEvent) that can be fired by any layer if an active editor should be closed. The table itself could listen to the event and close the editor. Doing that we would avoid that each layer needs to have a direct reference of the table.
What if I introduce another LayerEvent (CellEditorCloseEvent) that can be fired by any layer if an active editor should be closed
I thought of that too, but I guess it will create race conditions to fail, as if the editor can not be closed, actions should not apply. Like, don't scroll if the editor is invalid and can't be closed, as otherwise the open editor will stay while the rest of the table scrolls.