Added Position #1403

ataata107 · 2019-12-30T20:26:51Z

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
code is in uniquely-named feature branch and has no merge conflicts
PR is descriptively titled
ask @publiclab/is-reviewers for help, in a comment below
Insert-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!

ataata107 · 2019-12-30T20:30:43Z

@publiclab/is-reviewers

codecov · 2019-12-30T20:38:25Z

Codecov Report

Merging #1403 into main will increase coverage by 10.17%.
The diff coverage is 63.45%.

@@             Coverage Diff             @@
##             main    #1403       +/-   ##
===========================================
+ Coverage   55.11%   65.29%   +10.17%     
===========================================
  Files         117      132       +15     
  Lines        2344     2746      +402     
  Branches      360      438       +78     
===========================================
+ Hits         1292     1793      +501     
+ Misses       1052      953       -99

Impacted Files	Coverage Δ
examples/lib/scopeQuery.js	`18.51% <ø> (ø)`
src/Modules.js	`100.00% <ø> (ø)`
src/modules/WebglDistort/Module.js	`2.29% <0.00%> (ø)`
src/ui/SetInputStep.js	`12.90% <0.00%> (-1.39%)`	⬇️
src/modules/ColorHalftone/Module.js	`3.63% <3.63%> (ø)`
examples/lib/defaultHtmlStepUi.js	`11.05% <3.70%> (-1.15%)`	⬇️
examples/lib/intermediateHtmlStepUi.js	`11.11% <5.55%> (+0.94%)`	⬆️
examples/lib/insertPreview.js	`13.15% <20.00%> (-0.36%)`	⬇️
src/util/getImageDimensions.js	`20.00% <20.00%> (ø)`
src/util/isGif.js	`20.00% <20.00%> (ø)`
... and 100 more

anthony-zhou

Thanks for contributing. This looks great! I bet this feature could be useful to lots of people, but displaying it in a different way might be even better. Please take a look at the comment I made in the code. The purpose of the change I suggested is to show information on the page only when it's useful (there's no need to show position = NULL when the mouse is not on the photo). What do you think?

examples/lib/defaultHtmlStepUi.js

harshkhandeparkar · 2019-12-31T12:01:43Z

examples/lib/defaultHtmlStepUi.js

@@ -45,7 +45,7 @@ function DefaultHtmlStepUi(_sequencer, options) {
            <div class="panel-body cal collapse in">\
              <div class="row step">\
                <div class="col-md-4 details container-fluid">\
-                  <div class="cal collapse in"><p>' +
+                  <div class="cal collapse in"><p><p>POS_X= <span class="posx">NULL</span> POS_Y= <span class="posy">NULL</span></p>' +


Instead of POS_X and POS_Y, can it be more descriptive like

Cursor Position: (x-coord, y-coord)

examples/lib/defaultHtmlStepUi.js

ataata107 · 2019-12-31T22:30:50Z

ataata107 · 2019-12-31T22:37:08Z

Yes @anthony-zhou we shouldn't display NULL when cursor moves away. I have also put the text on top-right. Kindly Review

anthony-zhou

Looks good!

jywarren · 2020-01-02T17:34:06Z

Hi all! What if we put the position with a "mouse" icon ( https://fontawesome.com/v4.7.0/icon/mouse-pointer) for compactness, and in the card header, to the left of the trash button? Very cool!

…

On Wed, Jan 1, 2020 at 11:40 AM anthony-zhou ***@***.***> wrote: ***@***.**** approved this pull request. Looks good! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1403?email_source=notifications&email_token=AAAF6J3KMPMGXKTVN5IXQBDQ3TBP5A5CNFSM4KBP4Y4KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQO4PVA#pullrequestreview-337496020>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J7UIEXTF77MIVALEMLQ3TBP5ANCNFSM4KBP4Y4A> .

rishabhshuklax · 2020-01-05T19:23:17Z

How about a crosshairs icon in the card header which when clicked will trigger the function after which when hovered over the image it will give x,y cordinates in the same way it is currently showing rgb channels?

jywarren · 2020-01-06T21:34:39Z

Oh, that sounds cool! But, do we want it not to be on by default?

…

On Sun, Jan 5, 2020 at 2:28 PM Rishabh Shukla ***@***.***> wrote: How about a crosshairs icon <https://fontawesome.com/v4.7.0/icon/crosshairs> in the card header which when clicked will trigger the function after which when hovered over the image it will give x,y cordinates in the same way it is currently showing rgb channels? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1403?email_source=notifications&email_token=AAAF6J2WYTVL73O2T2MIGETQ4IXSLA5CNFSM4KBP4Y4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEID5RJI#issuecomment-570939557>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J2XXR4STEOBWROGWMTQ4IXSLANCNFSM4KBP4Y4A> .

rishabhshuklax · 2020-01-06T22:21:16Z

We don't need position data for all the modules this way we can keep both of the features (position and rgba channel on hover) and keep the UI clean and maybe we can also show the rgba data of image the same way by using a color-picker-icon...maybe?

jywarren · 2020-01-06T22:52:55Z

That's a cool idea! Yeah!

…

On Mon, Jan 6, 2020 at 5:21 PM Rishabh Shukla ***@***.***> wrote: We don't need position data for all the modules this way we can keep both of the features (position and rgba channel on hover) and keep the UI clean and maybe we can also show the rgba data of image the same way by using a color-picker-icon...maybe? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1403?email_source=notifications&email_token=AAAF6J6QE6IMS77AQHYKBJTQ4OVFZA5CNFSM4KBP4Y4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIG7UWQ#issuecomment-571341402>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6JZL2IEV7POS2S4X3Z3Q4OVFZANCNFSM4KBP4Y4A> .

ataata107 · 2020-01-07T22:43:10Z

I will do the necessary changes and will commit by tomorrow.

ataata107 · 2020-01-11T14:19:16Z

@jywarren @blurry-x-face Added the crosshair icon. It looks really cool now,
By the way I guess it should always be triggered on because we can get the idea of the RGB value and crosscheck it with the current position our cursor.

rishabhshuklax · 2020-01-11T14:38:03Z

@ataata107 Could you please attach a working GIF?

harshkhandeparkar

It's awesome! It even works with GIFs!

ataata107 · 2020-01-11T22:10:38Z

gitpod-io · 2020-07-07T19:02:50Z

jywarren · 2020-08-05T23:47:19Z

Hi, just tested this out in GitPod and although it looks amazing! I did notice the placement was a bit odd:

Can we try putting it in the top bar of each card, like this?

That way it'll work no matter what the page width is.

Thank you!!!!

jywarren · 2020-10-16T21:10:32Z

Hi @ataata107 would you be interested in making this last change and then we can merge it? Thank you!!!

ataata107 added 3 commits December 30, 2019 00:32

Display Coordinates

cbea607

Added position coordinates

9d8b994

Added position coordinates

a87cb39

anthony-zhou suggested changes Dec 30, 2019

View reviewed changes

examples/lib/defaultHtmlStepUi.js Show resolved Hide resolved

harshkhandeparkar requested changes Dec 31, 2019

View reviewed changes

Display position at top right

a734542

anthony-zhou approved these changes Jan 1, 2020

View reviewed changes

harshkhandeparkar approved these changes Jan 2, 2020

View reviewed changes

Merge branch 'main' into position

38e7561

harshkhandeparkar and others added 2 commits January 11, 2020 00:00

Merge branch 'main' into position

71d9713

Added crosshair icon

525880d

ataata107 requested a review from harshkhandeparkar January 11, 2020 14:19

Merge branch 'main' into position

004fdc7

harshkhandeparkar approved these changes Jan 11, 2020

View reviewed changes

harshkhandeparkar added the ready-to-merge label Jan 12, 2020

ataata107 mentioned this pull request Jan 21, 2020

Call for reviewers group and new co-reviewing workflow #656

Open

Merge branch 'main' into position

118690c

Merge branch 'main' into position

a98d7ae

harshkhandeparkar requested a review from a team as a code owner October 15, 2020 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Position #1403

Added Position #1403

ataata107 commented Dec 30, 2019 •

edited by harshkhandeparkar

Loading

ataata107 commented Dec 30, 2019

codecov bot commented Dec 30, 2019 •

edited

Loading

anthony-zhou left a comment

harshkhandeparkar Dec 31, 2019

ataata107 commented Dec 31, 2019

ataata107 commented Dec 31, 2019

anthony-zhou left a comment

jywarren commented Jan 2, 2020 via email

rishabhshuklax commented Jan 5, 2020

jywarren commented Jan 6, 2020 via email

rishabhshuklax commented Jan 6, 2020

jywarren commented Jan 6, 2020 via email

ataata107 commented Jan 7, 2020

ataata107 commented Jan 11, 2020

rishabhshuklax commented Jan 11, 2020

harshkhandeparkar left a comment

ataata107 commented Jan 11, 2020

gitpod-io bot commented Jul 7, 2020 •

edited

Loading

jywarren commented Aug 5, 2020

jywarren commented Oct 16, 2020

Added Position #1403

Are you sure you want to change the base?

Added Position #1403

Conversation

ataata107 commented Dec 30, 2019 • edited by harshkhandeparkar Loading

ataata107 commented Dec 30, 2019

codecov bot commented Dec 30, 2019 • edited Loading

Codecov Report

anthony-zhou left a comment

Choose a reason for hiding this comment

harshkhandeparkar Dec 31, 2019

Choose a reason for hiding this comment

ataata107 commented Dec 31, 2019

ataata107 commented Dec 31, 2019

anthony-zhou left a comment

Choose a reason for hiding this comment

jywarren commented Jan 2, 2020 via email

rishabhshuklax commented Jan 5, 2020

jywarren commented Jan 6, 2020 via email

rishabhshuklax commented Jan 6, 2020

jywarren commented Jan 6, 2020 via email

ataata107 commented Jan 7, 2020

ataata107 commented Jan 11, 2020

rishabhshuklax commented Jan 11, 2020

harshkhandeparkar left a comment

Choose a reason for hiding this comment

ataata107 commented Jan 11, 2020

gitpod-io bot commented Jul 7, 2020 • edited Loading

jywarren commented Aug 5, 2020

jywarren commented Oct 16, 2020

ataata107 commented Dec 30, 2019 •

edited by harshkhandeparkar

Loading

codecov bot commented Dec 30, 2019 •

edited

Loading

gitpod-io bot commented Jul 7, 2020 •

edited

Loading