bwu-dart / bwu_datagrid

A data-grid Polymer element in Dart
MIT License
74 stars 26 forks source link

Grid does not handle re-attaching well #97

Open skalkin opened 10 years ago

skalkin commented 10 years ago

If you attach-detach-attach the grid (common scenario when a control is in a docking window), the following exception is thrown:

Breaking on exception: Bad state: Future already completed

zoechi commented 10 years ago

Hi Andrew, thanks for reporting. I'll look into it. Do you have a stack trace?

skalkin commented 10 years ago

Thanks for getting back so quickly :) Yes, the exception comes from the BwuDatagrid.attached method when it's being attached for the second time (_setupCompleter.complete() line). Here is the relevant stack trace:

... DockManager.requestUndockToDialog() Dialog.Dialog() Dialog._initialize() _ChildNodeListLazy.add() Node.append() BlinkNode.$appendChild_Callback() BwuDatagrid.attached() _AsyncCompleter.complete()

zoechi commented 10 years ago

Great, thanks!

zoechi commented 10 years ago

I couldn't reproduce see https://github.com/bwu-dart/bwu_datagrid_playground/blob/master/web/issue97_re_attach/app_element.dart#L63 Can you please check what you do differently?

I had another issue with broken layout https://raw.githubusercontent.com/bwu-dart/bwu_datagrid_playground/master/web/issue97_re_attach/ressources/datagrid_broken_layout.png

It's the same I see in #87

zoechi commented 10 years ago

As a workaround for the layout issue call after re-attaching

grid.setColumns = (columns);
skalkin commented 10 years ago

Hi Günter, indeed, the end result is the broken layout, but I'm also getting the exception if the "break on exceptions" flag is on. If I choose to continue, I end up with broken layout.

zoechi commented 10 years ago

That helps, I will look into it again. Maybe this is again an exception swallowed by Polymer.

zoechi commented 10 years ago

I still couldn't reproduce the exception and I'm pretty sure you initialize the grid in a different way than my example. While examining the code your stack trace points to, I found a bug that could lead to your problem. I published a new release of BWU Datagrid with a fix. Can you please verify whether this prevents the exception being thrown? You might still need to pass the column definition again after re-attaching the grid to fix the layout though (see workaround above)

skalkin commented 10 years ago

Thanks a lot, yes it indeed does fix the issue with the exception (I have fixed it in a similar way locally, so now I can switch back to the pub version :)). Broken layout issue still remains, but it looks like you were already aware of it. Indeed, passing the column definition restores the layout, but I'm not aware of an easy way to intercept element detaching/attaching. I am using the (awesome) dock_spawn library for window docking, and bwu_datagrid now works nice except for the layout issue and also not properly auto-resizing when the grid's size changes.

zoechi commented 10 years ago

I added an BwuAttached event, that should simplify remove/re-attach. I updated the example (see link above) to show how to use it.

The example also shows how to update the grid when the size of the container changes.
This only works when the container fires an event on resize (see example). Please let me know if this solves your problems.

skalkin commented 10 years ago

Thanks a lot, that helped! The minor unfortunate side effect of re-initializing columns is that the current viewport gets reset.

Also, just wondering why "setColumns" is a property and not a method, like its name suggests?

zoechi commented 10 years ago

What do you mean by viewport gets reset? Customizations like column resize/reordering get lost or are there other effects? I need to investigate further why the layout breaks after re-attach in the first place.

This are the names from the JS source. I tried to avoid refactoring before the entire code base is working properly. There is still work to do. Unfortunately this will be breaking changes for users but the changes will be only renames of members and change between function and getter/setter nothing structurally. I'll try to track and document the changes properly so it should be easy for users to apply the necessary changes.

skalkin commented 10 years ago

By "viewport gets reset", I meant that the currently observable section of the grid (where you scrolled you) gets reset and after the "setColumns = getColumns" assignment you end up looking at the row #1, column #1. Column sizes are column order do not change.