fix GridItem calcXY error calculation

[xiaobao66]

Fixed: #918

Issue: GridItem calcXY calculation is wrong.

Solution: Change the calculation of the left and top value. The new calculation is left = colWidth * x + margin * x + containerPadding

[STRML]

Indeed #918 is a real bug. If we can get codesandbox working on PRs it would be great to try this out, otherwise we can pull & compare.

[STRML]

@xiaobao66 could you please add a test that fails on master?

[STRML]

Verified this fixes the issue locally with the example grid and:

          containerPadding={[0, 0]}
          margin={[100, 10]}

However we need a regression test.

[xiaobao66]

@STRML Sorry, I don't know how to write a regression test for this fix. Do you mean add a example in the test directory?

[STRML]

Yes, something in test/spec that fails under the old code but succeeds under the new.

[xiaobao66]

Yes, something in test/spec that fails under the old code but succeeds under the new.

Sorry, I don't know what fails under the old code. I didn't change test/spec and my pr has passed the test.

[STRML]

The intention here is to write a new test that ensures that your fix doesn't get removed in the future and to ensure that the new behavior we've just identified as "good" stays good.

[xiaobao66]

@STRML I find test/spec only test function, but GridItem.jsx is a react component. This is my first time to commit pr, so I don't know how to add a test for react component. Could you give an example?

[STRML]

Yeah, we do it in https://github.com/mzabriskie/react-draggable/blob/master/specs/draggable.spec.jsx using react-dom/test-utils & react-test-renderer. Enzyme is also an option.

[xiaobao66]

Ok, I will look at it first and then spend time adding the test

[xiaobao66]

@STRML I have added a test case for the fix.But it shows Run actions/labeler@v2 failed,

##[error]HttpError: Resource not accessible by integration ##[error]Resource not accessible by integration ##[error]Node run failed with exit code 1

what does it mean?

[github-actions[bot]]

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 7 days