https://github.com/publiclab/image-sequencer/pull/1187 introduced a nice new tool for selecting pixel coordinates by clicking the main image. However, we need a UI test to protect this, and I'd like to ensure we also have:
We'd love help with this, building on top of the commits in https://github.com/publiclab/image-sequencer/pull/1187 so that we can incorporate that excellent addition as well! Thanks!
For an example of this kind of test, see: https://github.com/publiclab/image-sequencer/blob/main/test/ui-2/test/ColorPicker.test.js
Ok i will try this馃槉
Actually @jywarren sir i went through the entire conversation held in #1187 and i think that i have already did this in #1804 . with all the requirements satisfied like -
One more thing sir how do i push the code to the existing pull request won't there be any permission conflict there?.
@jywarren i have tried to write a test for this and everything is working fine except for only one thing. But don't know how to tackle that, could you please give some suggestions.
see -

the problem is when we select the image to click on it we have 2 images get selected and image.hover will hover on the first element it found hoverable.
see the documentation -

so how do we reach to second image ????
@jywarren sir after this the whole test will be completed
Hi, @vivek-30 my apologies for replying slowly. Just to catch up:
I think you're right that you've solved #1187 in #1804 however just to confirm i think you did it slightly differently - by defining a coordinate-input type rather than having an id in the form "id": "x-coord" as @aashna27 did?
If so we should be OK however i see that in #1187 @aashna also enabled this type of input for 4 modules:
So i think we have to add it to those too?
Finally, yes, the test is the last big piece. Let me look to see what to do...
For the test, are you working in a pull request you can link us to? So we can refer to specific lines of code?
I'm not sure which click event is getting the wrong image - i think we should be able to use more specific CSS to click but can you highlight:
then hopefully we can disentangle it! Thanks!
Hi, @vivek-30 my apologies for replying slowly. Just to catch up:
I think you're right that you've solved #1187 in #1804 however just to confirm i think you did it slightly differently - by defining a
coordinate-inputtype rather than having anidin the form"id": "x-coord"as @aashna27 did?If so we should be OK however i see that in #1187 @aashna also enabled this type of input for 4 modules:
- CanvasResize
- Colorbar
- Crop
- FisheyeGL
- GridOverlay
- Overlay
- TextOverlay
So i think we have to add it to those too?
Finally, yes, the test is the last big piece. Let me look to see what to do...
No Problem @jywarren sir , I understand that you have lot of work to do and finding time for all such things is very hard.
Taking about the issue, yes you are absolutely right i did it in a bit different manner as that issue is targeting only ease in adjusting text on the image. @aashna27 did it for #1187 which solely focuses on input-coordinates. No doubt that her approach is to the point and amazing . what we can do is just complete this test and then we can merge her PR. later i will modify the code for above constraints like (crosshairs icons , fill coordinates on click) and what i have added in #1804. thanks.
For the test, are you working in a pull request you can link us to? So we can refer to specific lines of code?
I'm not sure which click event is getting the wrong image - i think we should be able to use more specific CSS to click but can you highlight:
- which line is causing the issue
- which image it's supposed to click
- which image it's actually clicking
then hopefully we can disentangle it! Thanks!
sorry sir if i have confused you . i was talking about the PR #1187 , i thought after completing the test i have to add the commit in this PR . so thats why i asked "how will i add a commit to someone else PR with appropriate authority".
BTW i will try this again with some different approach 鈽猴笍
Hi, sorry, i may still misunderstand, but if you can add the test in its own PR that's fine - in theory your own solution should fulfill it, right?
Merging @aashna27's code would be great, but we don't want 2 different systems, and your seems fine -- do you see an added benefit of also merging @aashna27's code?
I'm sorry I didn't catch that the two PRs overlapped. We were holding @aashna27's code until we had the right testing infrastructure, and I feel bad that we are considering not merging it now. That's on me - apologies to @aashna27 and yourself for the mixup and I appreciate both your work and time, both of you!