BTMorton / angular2-grid

A drag/drop/resize grid-based plugin directive for angular2
https://bmorton.co.uk/angular/
MIT License
354 stars 159 forks source link

GridItems getting auto aligned to incorrect position #268

Open smkart opened 6 years ago

smkart commented 6 years ago

Hi,

Facing issue with loading girdItem back into saved position.

Workflow :

  1. Add items to grid
  2. Save the grid as JSON
  3. Load the grid back with saved grid items

Grid items which is added second it moving to incorrect position

image

As you see in the image , Top item with "Line chart" is grid item 1 and "Table chart" is grid item 2 ( Table chart is added into grid after Line chart )

GirdItems Configuration as per the above postion in the image:

GridItem1 -> col : 8 and Row: 1 GridItem2 -> col : 17 and Row: 16

Grid is saved as JSON with the Above grid items configuration

Now I am moving GridItem2 just above GridItem1 as shown below:

image

GirdItems Configuration as per the above postion in the image:

GridItem1 -> col : 8 and Row: 16 GridItem2 -> col : 16 and Row: 1

Saved the above position into JSON

Now I am loading the same JSON , Expected to load it with below config Expected: GridItem1 -> col : 8 and Row: 16 GridItem2 -> col : 16 and Row: 1

Actual ( Bug ) GridItem1 -> col : 8 and Row: 1 GridItem2 -> col : 17 and Row: 16

I walked into code and below are my findings in code

For each grid item below function is called twice:

On first call below is the call stack: NgGridItems.js -> setConfig(config)

setConfig(config) contains below statement

image

this.added is false by default so the updateItem(this) is skiped on first call and grid is set to expected position

But, On Second call below is the call stack:

NgGridItems.js -> setConfig(config)

setConfig(config) -> this.added as true and it called updateItem(this);

Inside NgGrid.js updateItem(this) -> this._cascadeGrid()

this._cascadeGrid() contains below code image

Now the "var newPos"it set with col as expected but row to lowest , where lowest is 1

"ShouldSave" is true by default so then item.savePosition(newPos) -> sets row to be lowest as 1

This change is newPos causing the issue

I have the below config on my gird

gridConfig: NgGridConfig = { 'margins': [2], 'draggable': false, 'resizable': false, 'max_cols': 0, 'max_rows': 0, 'visible_cols': 0, 'visible_rows': 0, 'min_cols': 1, 'min_rows': 1, 'col_width': 30, 'row_height': 20, 'cascade': 'up', 'min_width': 300, 'min_height': 200, 'fix_to_grid': false, 'auto_style': true, 'auto_resize': false, 'maintain_ratio': false, 'prefer_new': false, 'zoom_on_drag': false, 'limit_to_screen': true };

I hope I have giving enough information , Please feel free to ask more details

Thanks Mani

smkart commented 6 years ago

And one more finding is , The issue occurs only when the cascade is set to "up" As you seen in code with switch block condition set to "up" .

When I set cascade "off" everything works but usability is worse

Thanks Mani

BTMorton commented 6 years ago

So, I can think of a couple of issues at play here. The fact it's calling setConfig twice is one. Another is that the grid should do a cascade after items are added so that they end up in the correct place, which means that the first item added will automatically be pushed to the top of the grid, in your case resulting in the wrong layout.

I need to put some thought into the grid cascade process and when it actually triggers an update or save of the item positions and that will come most likely with a new algorithm for that side of things. For the time being, I can make sure that that cascade call is async and leave enough time for all other items to be added into the grid.

I think the reason that the setConfig method is being called twice with the same properties is due to the change detection I have in place for the config and the KeyValueDiffer I'm using. I'll need to do some more research into that side of things to see if I can resolve the issue.

I'll keep you posted with anything I find.

BTMorton commented 6 years ago

So, I've made some improvement to that end, adding a delay before cascades and optimising the change detection flow. Should help somewhat with the issues that you're facing. What I'll do is release those changes as a beta version so that you can give them a test. It'll also include some performance work that I've been doing recently to improve many of the underlying grid mechanisms, so any feedback or issues you find with that would be appreciated too.

BTMorton commented 6 years ago

Published changes as 3.0.0-beta.0. If you could install and let me know if you have any issues, it'd be much appreciated 👍

smkart commented 6 years ago

Thanks for the fix, And I also confirm that your fix works perfectly.

There is some performance issue while moving elements inside gird, I could see some lack of smoothness ( which spoils user experience ). If you can find anything regarding this would be much helpful

But with 3.0.0-beta.0 the issue got fixed , thanks again for the timely response

Regards, Mani

sconix commented 6 years ago

We also have this issue and I can confirm that 3.0.0-beta.0 fixes the issue, but however it introduces a new one. With the beta version the items in the grid move prematurely when dragging items. I.e. when I drag item to a empty space the above item already moves away even the dragged item is not over the item (i.e. hovering it over empty space not the above item). Its like the items are moved away one row too early. If the dragging is released the above item returns to its place. This new bug does not seem to be related to the cascade setting.

smkart commented 6 years ago

Hi,

3.0.0-beta.0 seems fixed the issue but as @sconix mentioned I am facing some new issue

Issue:

  1. When I resize the 2nd widget first one goes below the second and come back immediately after resize complete
    image
  2. There seems like some script error which hangs my entire application , I am not even able to open developer console, And it doesn't seems like throwing any error to console as well
  3. I request your to change the resolution of your monitor and change the zoom percentage, Because when my chrome zoom set's to default zoom then the fix seems broken ( It works when my zoom is less than the default example: it works with 80% and not with 110% )

Please help me by solving above issue

Thanks, Mani

sconix commented 6 years ago

We are getting close to release (in the end of the year) and would love to get rid of couple hacks that we are using with the current release. The 3.0.0-beta.0 looks promising (i.e. no hacks needed), but as said it seems to have couple small grid dragging/resize bugs. @BTMorton if you need help I can try to have a look of the beta changes and find possible reason to this once I have time.

smkart commented 6 years ago

@BTMorton Please help by fixing the issue , we are in some kind of urgent to get this done as quick as possible

Thanks Mani

smkart commented 6 years ago

Hi,

@BTMorton Any update in this issue please ? I tired exploring code to fix , but couldn't get exact fix

So please look into this issue for fix ASAP

Thanks, Mani Applied Materials

smkart commented 6 years ago

Hi @BTMorton

Please help us to fix the issue ,It is so critical in our project

Thanks, Mani

smkart commented 6 years ago

@sconix Do you have any hack/workaround for this issue ? It will be helpful If you can give me some workaround

Thanks Mani

sconix commented 6 years ago

Not yet, but we have also reached the point were we need to fix this since there does not seem to be any alternatives. So I will be most likely looking into fixing this during this week or at the very latest next week.

SairamPotta commented 3 years ago

I am also facing the same issue. @sconix and @BTMorton can you please provide a solution to this issue. Currently using 3.0.0 version