Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Column resize doesn't properly work on mobile #705

Closed
daniela-mateescu opened this issue Nov 9, 2023 · 11 comments · Fixed by #706
Closed

Column resize doesn't properly work on mobile #705

daniela-mateescu opened this issue Nov 9, 2023 · 11 comments · Fixed by #706
Labels
bug regression issue that was "fixed" but got reopened later on

Comments

@daniela-mateescu
Copy link
Contributor

Dragging to resize a column on mobile doesn't work on a new project using your project as a library

But If I check out your project (on the master branch) and I run your examples (using yarn site-dev-server ) from there the resize is working ok on mobile

Expected Behavior

After touch down, when touch moves, the resize line should move and when touching up the column should get resized

Current Behavior

When dragging for resizing a column on mobile, the resize line doesn't move and when touch up, the column doesn't resize. If you tap second time the column gets resized.

Steps to Reproduce (for bugs)

  1. You should open the following link in chrome
    https://codesandbox.io/s/exciting-meitner-9qjjqf?file=/src/index.js

  2. Use the mobile mode of the chrome dev tools
    image

  3. Resizing a column doesn't work

Your Environment

  • Version used: latest (2.0.2)
  • Browser Name and version: Chrome Version 119.0.6045.123
@pradeepnschrodinger
Copy link
Collaborator

Thanks for filing.
This is a regression; Resizing in mobile view or touch devices works in v1.2.x but not in v2.0.x 😞

@pradeepnschrodinger pradeepnschrodinger added the regression issue that was "fixed" but got reopened later on label Nov 9, 2023
@daniela-mateescu
Copy link
Contributor Author

daniela-mateescu commented Nov 10, 2023

Ok. Thank you! But do you have any ideea why if

  • I clone your project (on the master branch),
  • I add on the ResizeExample.js for each ResizeCell touchEnabled={true}
  • I run your examples (using yarn site-dev-server ),

The resize is working ok on mobile?
https://www.youtube.com/watch?v=ssChIDgGRtE

(Also the reorder column example is working ok on the locally checked out project with touchEnabled={true} )

@pradeepnschrodinger
Copy link
Collaborator

Interesting.

We're no longer passing down the touchEnabled prop to the cell renderers within FDT.
But the resizer plugin <ResizeCell /> still expects it, making it not work for touch devices.

I add on the ResizeExample.js for each ResizeCell touchEnabled={true}

So a quick workaround is to explicity pass down the prop to the cell renderer from the user's side, like what you did.

The actual fix likely involves FDT passing it down instead, like what I see for v1.2.x.

But do you have any idea why if the resize is working ok on mobile?

I unfortunately can't figure out why there's a difference in behavior with the local webpack server vs the one in hosted in codesandbox.
Even the v1.2.x version doesn't seem to work in codesandbox.
cc: @AmanGupta2708 , @karry08 , @abhisheksingh2410

On a side note, I'm also seeing an issue where trying to resize via touch results in the table being scrolled in the background.
It would be nice to check if both of these issues are stemming out from the same underlying problem.

@cristian-spiescu
Copy link

Resizing in mobile view or touch devices works in v1.2.x

In our opinion this is false. This is actually the exact reason why we are trying a migration towards v2, hoping to fix the behavior on mobile.

@pradeepnschrodinger
Copy link
Collaborator

@daniela-mateescu, @cristian-spiescu , I put a potential fix for this at #706, with a couple of other touch related issues I saw along the way.

I did a quick round of smoke testing by forking your example (see https://codesandbox.io/s/fixed-data-table-2-touch-resizing-s22nn7) and it seems to work well.

The fix can be grabbed by specifying "fixed-data-table-2": "https://github.com/schrodinger/fixed-data-table-2#fix-touch-handlers-prerelease", in your package.json file.
It would be awesome if you guys can test it with your project, and let us know if it works.

@cristian-spiescu
Copy link

Thanks a lot! We'll give it a shot Monday and get back to you. Have a nice weekend.

@daniela-mateescu
Copy link
Contributor Author

Hello! Thanks a lot for this fix! We confirm that we have tested and that the resize and reorder works now on our application (on mobile) with this your new prerelease version. We are looking forward for a release version.

@cristian-spiescu
Copy link

@pradeepnschrodinger do you have an idea when is a next version available, containg this fix? Maybe 2.0.3?

@pradeepnschrodinger
Copy link
Collaborator

Thanks for the testing out the fix and letting us know it works.

The next release containing this fix is likely v2.0.3, and should be released within a day or two.
I'll comment here and on the PR once it's done.

@pradeepnschrodinger
Copy link
Collaborator

Fix released with v2.0.3.

cc: @daniela-mateescu , @cristian-spiescu

@cristian-spiescu
Copy link

Thanks a lot @pradeepnschrodinger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug regression issue that was "fixed" but got reopened later on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants