Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

GridItems getting auto aligned to incorrect position #268

Open
smkart opened this issue Sep 20, 2017 · 14 comments
Open

GridItems getting auto aligned to incorrect position #268

smkart opened this issue Sep 20, 2017 · 14 comments

Comments

@smkart
Copy link

smkart commented Sep 20, 2017

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 smkart changed the title GridItem auto aligned to incorrect position GridItems getting auto aligned to incorrect position Sep 20, 2017
@smkart
Copy link
Author

smkart commented Sep 21, 2017

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
Copy link
Owner

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 added a commit that referenced this issue Oct 22, 2017
Add a delay before cascading when adding/removing/updating items #268
BTMorton added a commit that referenced this issue Oct 22, 2017
Add a delay before cascading when adding/removing/updating items #268
@BTMorton
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

smkart commented Nov 2, 2017

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
Copy link
Contributor

sconix commented Dec 3, 2017

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
Copy link
Author

smkart commented Dec 14, 2017

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
Copy link
Contributor

sconix commented Dec 14, 2017

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
Copy link
Author

smkart commented Dec 21, 2017

@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
Copy link
Author

smkart commented Jan 8, 2018

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
Copy link
Author

smkart commented May 14, 2018

Hi @BTMorton

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

Thanks,
Mani

@smkart
Copy link
Author

smkart commented May 14, 2018

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

Thanks
Mani

@sconix
Copy link
Contributor

sconix commented May 14, 2018

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
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants