Closed aw1875 closed 3 months ago
Thanks for the PR!
If I understand correctly, this is only showing the timestamps, not actually using it, right?
Maybe we should change the puppeteer mouse.move()
event to use CDP Input.dispatchMouseEvent()
directly, which supports a timestamp: https://chromedevtools.github.io/devtools-protocol/tot/Input/#method-dispatchMouseEvent
That's correct it's just for show. However, I wouldn't be against digging more into the Input.dispatchMouseEvent
docs to use the generated timestamps in puppeteer if you think it would make sense for me to do.
@Niek just checking back in so I don't lose track of this pr. Did you want me to make any changes?
In my opinion this only makes sense if the timestamps are used, meaning we need to switch to manual Input.dispatchMouseEvent()
calls.
In my opinion this only makes sense if the timestamps are used, meaning we need to switch to manual
Input.dispatchMouseEvent()
calls.
Honestly, I don't really see any scenario where this would work unless I'm also making changes to functions like move
, moveTo
, etc. However, something like this could technically take that approach (this is kinda hacky but you get the idea):
const tracePath = async (
- vectors: Iterable<Vector>,
+ vectors: Iterable<Vector | TimedVector>,
abortOnMove: boolean = false
): Promise<void> => {
...
- await page.mouse.move(v.x, v.y)
+ if ('timestamp' in v) {
+ await getCDPClient(page).send('Input.dispatchMouseEvent', {
+ type: 'mouseMoved',
+ x: v.x,
+ y: v.y,
+ timestamp: v.timestamp
+ })
+ } else {
+ await page.mouse.move(v.x, v.y)
+ }
...
Or, we could always use the dispatchMouseEvent with something more along the lines of:
const tracePath = async (
- vectors: Iterable<Vector>,
+ vectors: Iterable<Vector | TimedVector>,
abortOnMove: boolean = false
): Promise<void> => {
...
- await page.mouse.move(v.x, v.y)
+ const dispatchParams: Protocol.Input.DispatchMouseEventRequest = {
+ type: 'mouseMoved',
+ x: v.x,
+ y: v.y,
+ }
+
+ if ('timestamp' in v) dispatchParams.timestamp = v.timestamp
+
+ await getCDPClient(page).send('Input.dispatchMouseEvent', dispatchParams)
...
Yes, this is what I was thinking of. Although it's probably better to have the CDP client initialised once in order to avoid creating thousands of clients.
Good call, my brain didn't even process that I was calling that in a loop when I sent that code block
Awesome, thanks! This LGTM, but @bvandercar-vt can you review this as well?
Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.
Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.
Did you run yarn lint
? Should fix the formatting
Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.
Did you run
yarn lint
? Should fix the formatting
Yeah that was post yarn lint
(plus husky should be running the linter automatically anyway)
Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.
Did you run
yarn lint
? Should fix the formattingYeah that was post
yarn lint
(plus husky should be running the linter automatically anyway)
Ah yeah yarn lint
doesn't fix this whitespace issue on my end either:
Definitely a downside of ts-standard
then. We could add prettier
as a dep if the repo owners are interested.
Update: Not sure if the repo owners will go for it, but I wanted to see how it looked-- made a prettier PR: https://github.com/Xetera/ghost-cursor/pull/142
Not sure what happened there with the merge conflicts not getting all the changes the first time around and with the whitespace changes on the second one (husky ran properly), if one of you could fix the whitespace I'd appreciate it.
Did you run
yarn lint
? Should fix the formattingYeah that was post
yarn lint
(plus husky should be running the linter automatically anyway)Ah yeah
yarn lint
doesn't fix this whitespace issue on my end either:
Definitely a downside of
ts-standard
then. We could addprettier
as a dep if the repo owners are interested.Update: Not sure if the repo owners will go for it, but I wanted to see how it looked-- made a prettier PR: #142
super annoying, still a little what caused the whitespace to begin with as I turned off any formatters I have when working in this repo but not sure what else I can do besides manually fix it
Should be good now, really odd but oh well
@aw1875 PR looks good! Thanks for implementing those changes!
Thanks a lot, merged now!
Take two on a PR I did a few years ago (https://github.com/Xetera/ghost-cursor/pull/42).
This version does essentially the same thing as the original PR but with a more elegant approach by utilizing the options that can be passed to the path function and calculating the time to take using the Trapezoidal rule.
I'm open to any suggestions/changes so please let me know.