Skip to content

Conversation

@nulen
Copy link

@nulen nulen commented Oct 25, 2017

There was a problem with positioning dragged widgets if there were larger number of columns and cumulated diff between real value and rounded one. (e.g. cellWidth is calculated to be 19.5px and there are 30 columns. Further we go, larger the mistake with positioning widgets)

Description

Updated cellWidth method to not round cellWidth value. It was causing problem for grids with larger number of columns and uneven cell width calculated.

Updated cellWidth method to not round cellWidth value. It was causing problem for grids with larger number of columns and uneven cell width calculated. (e.g. cellWidth is calculated to be 19.5px and there are 30 columns. Further we go, larger the mistake with positioning widgets)
@coveralls
Copy link

coveralls commented Oct 25, 2017

Coverage Status

Coverage remained the same at 69.59% when pulling 84c562d on nulen:patch-2 into 57c6511 on gridstack:develop.

@radiolips
Copy link
Member

This is an exact value and not a percentage. I'd imagine in every case you'd want this to return the rounded value. Why is this causing problems?

@radiolips radiolips closed this Nov 13, 2019
@adumesny
Copy link
Member

if we use cellWidth() * n, then the rounding will mess things up. I agree with him, taking a look.

@adumesny
Copy link
Member

Created a sample for #1053 with 30 columns, but not seeing issues in that PR (issue rounding cellWidth() with large # of columns). I was able to move and resize and things lined up as expected even when total width / 30 was close to .5 off
@nulen please update with info.

@adumesny
Copy link
Member

@nulen closing for now I don't see the need for this 'fix' - please re-open and update with more info (see my comment and sample app)

@adumesny adumesny closed this Nov 19, 2019
@bpwalker
Copy link

bpwalker commented Nov 22, 2019

@adumesny A breaking example is using a large number of columns, say 100, and try and move a widget that has a larger x position (far right of the grid). In dragOrResize, ui.position.left / cellWidth will result in a huge error that will result in the widget moving significantly even if you've barely touched it.

@adumesny adumesny mentioned this pull request Jun 25, 2020
@adumesny adumesny reopened this Jun 25, 2020
adumesny added a commit to adumesny/gridstack.js that referenced this pull request Jul 5, 2020
* cellWidth() no longer round things off
(I was able to see issue with 60 columns and dragging itelf on right and would not relocate unless moved a lot futher).
adumesny added a commit that referenced this pull request Jul 5, 2020
TS: fix for #810 many columns roundoff error
@adumesny
Copy link
Member

adumesny commented Jul 5, 2020

thank you, I ended up checking in the fix for 2.x

@adumesny adumesny closed this Jul 5, 2020
adumesny added a commit to adumesny/gridstack.js that referenced this pull request Jul 5, 2020
adumesny added a commit that referenced this pull request Jul 5, 2020
TS: more fix for #810 many columns roundoff error
adumesny added a commit to adumesny/gridstack.js that referenced this pull request Jul 27, 2020
* forgot to update the test case for gridstack#810 changes
* also found getCellFromPixel() was incorrect in rounding as well or delaing with margins
(which don't affect cell size, just content)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants