search

deshiknaves / caret-pos

Locate the current position of the caret. A plain JavaScript version of Caret.js.
MIT License
120 stars 15 forks source link

Allow getting offset from custom position in element #3

Closed imsnif closed 6 years ago

imsnif commented 6 years ago

Hi @deshiknaves - I hope you'd be up for this additional feature. So, previously the lib had this feature only for textArea, and the API was IMO a little confusing. I added this feature for both textArea and contentEditable with the same API (passing {customPos: <pos>}).

Let me know if you'd like me to make any changes.

deshiknaves commented 6 years ago

Hey @imsnif, that looks like a good addition to me. I'll merge it in now and publish it a bit later on this evening.

deshiknaves commented 6 years ago

@imsnif might have to delay the release till tomorrow. The build is passing the CI, but for some reason it's not on my machine. I need to investigate it a bit further. I'll keep you posted.

PhantomJS 2.1.1 (Mac OS X 0.0.0) caret-pos Input should correctly get the caret offset with a custom position FAILED Expected 16 to be 19. src/caret.spec.js:502:30 Expected 20 to be 22. src/caret.spec.js:503:28 PhantomJS 2.1.1 (Mac OS X 0.0.0) caret-offset Editable should correctly get the caret offset FAILED Expected 16 to be 19. src/caret.spec.js:597:30 Expected 31 to be 37. src/caret.spec.js:598:28 PhantomJS 2.1.1 (Mac OS X 0.0.0) caret-offset Editable should correctly get the caret offset with a custom position FAILED Expected 16 to be 19. src/caret.spec.js:611:30 Expected 27 to be 31. src/caret.spec.js:612:28

imsnif commented 6 years ago

On my machine it passes, but I'm on a linux box like the build machien... so could be OS related I suppose? Let me know if I can help with anything.

deshiknaves commented 6 years ago

I now remember why I originally didn't include tests for offset. They weren't reliable on all platforms, even with css resets. Last time I only check the pipeline where they are fine, but it's been broken on my machine since then.

imsnif commented 6 years ago

Ahh, damn. That makes sense. May I recommend puppeteer https://pptr.dev/?

It will likely increase test running time, but should solve the multi-platform issue, if I understand it correctly.

deshiknaves commented 6 years ago

Yep, puppeteer is great. However, I don't think it's going to fix this issue. I'll give it a go tomorrow evening.

deshiknaves commented 6 years ago

@imsnif this is now resolved. Yep, puppeteer was a lot closer — thanks for that. Also had to add css resets. But consistent now. Will publish it shortly.

imsnif commented 6 years ago

Great! Thanks for this.