-
Notifications
You must be signed in to change notification settings - Fork 26
feat: add touch support for reorderable and resizable features #181
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
base: master
Are you sure you want to change the base?
Conversation
Enabled touch event handling in key directives such as draggable, resizeable, and long-press, ensuring compatibility with touch devices. Introduced utility functions for identifying touch events and extracting event coordinates. Updated styles to enhance usability on touch devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx a lot for your contribution. This is highly appreciated and it works very well.
Beside the two very minor comments, this needs some automated testing.
There are resize and drag and drop tests in the playwright directory.
Would be awesome if you can add tests using touch events there.
As our git-lfs setup is weird due to github fork restrictions, you won't be able to push screenshots. So please put await si.runVisualAndA11yTests('***');
where needed but comment those lines, so I can later create screenshots and push them on your behalf after merging this PR.
Since this project currently has very poor unit testing, I think it is fine omitting unit test for now (unless you want to do it). We will introduce them later.
projects/ngx-datatable/src/lib/directives/resizeable.directive.ts
Outdated
Show resolved
Hide resolved
projects/ngx-datatable/src/lib/directives/long-press.directive.ts
Outdated
Show resolved
Hide resolved
Sure I'll look into Playwright some time this week. I did find an open issue talking about support for Touch Events: microsoft/playwright#2903 |
I think in that case you can also skip adding playwright tests. Since we anyway have to construct the event manually unit tests will do. But there is now one existing test failing (
|
Just one more thing, my previous comment broke the code on Firefox. Seems like the desktop version does not support |
I have updated the PR with the MouseEvent change, but sadly the latest changes completely broke the PR. I'll have to look over, and re-implement some stuff. And I still haven't found any option for playwright touch events. |
What kind of change does this PR introduce? (check one with "x")
What is the current behavior? (You can also link to an open issue here)
Columns can't be resized or reordered.
What is the new behavior?
Columns can be resized and reordered.
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: