Shorten UI code with new `$step` method #710

harshkhandeparkar · 2019-01-24T06:35:05Z

Fixes #833

Refactors the UI code by using new methods $step and $stepAll.

instead of

$(step.ui.querySelector('query')).show().hide();
$(step.ui.querySelectorAll('q2')).show().hide();

Use

$step('query').show().hide();
$stepAll('q2').show().hide();

Also with (what I call) selector chaining:

$step('query').show().hide().elemAll('q2').show().hide();

How does it work

Import the scopeSelector and scopeSelectorAll methods from lib/scopeQuery.js
Call the methods with scope(step.ui in this case)

var scopeQuery = require('./scopeQuery');

var $step = scopeQuery.scopeSelector(step.ui),
  $stepAll = scopeQuery.scopeSelectorAll(step.ui);

This will return an object with a constructor which returns a jQuery object (from inside the scope) but with new elem and elemAll methods.

This method can reduce the frequent occurrences of $(step.ui.querySelector('q')) especially with selector chaining.

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/reviewers and @publiclab/is-reviewers for help, in a comment below

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

Thanks!

Signed-off-by: tech4GT <varun.gupta1798@gmail.com> dist update Revert "dist update" This reverts commit 9ee2a98.

harshkhandeparkar · 2019-01-24T18:53:18Z

@publiclab/is-reviewers please review.

harshkhandeparkar · 2019-01-24T18:59:27Z

I'm going to change the names to scopeQuery tomorrow 😋

harshkhandeparkar · 2019-01-25T06:32:59Z

@publiclab/is-reviewers I have made the changes and rebased. Please review

harshkhandeparkar · 2019-02-28T11:08:15Z

@jywarren can you please merge my PRs before the other ones since I will not be able to touch my PC until 16th of april. I don't know if I will get time past that date as well.

jywarren · 2019-03-04T15:21:55Z

Hi, sorry I missed this way back. I'm wondering if we could pass this with the utility functions we make available to steps:

https://github.com/publiclab/image-sequencer/blob/main/CONTRIBUTING.md#draw

What do you think? I like the idea of a convenience method, though! Thanks!

jywarren · 2019-03-04T15:22:39Z

Also, i think it'd be best to have a test -- no rush on this, i know you're busy, but just to reply (finally, apologies again!)

harshkhandeparkar · 2019-03-04T15:24:34Z

No need for an apology. Also I think tests and docs can be added in a followup by somebody else since I'm not very experienced with jasmine and also someone can add to the docs(maybe a newcomer) very easily. What do you think?

jywarren · 2019-03-04T15:31:23Z

Could you open up a new issue linking here and proposing adding tests built on this PR? We could then merge them together. Thanks!

…

On Mon, Mar 4, 2019, 10:25 AM Harsh Khandeparkar ***@***.***> wrote: No need for an apology. Also I think tests and docs can be added in a followup by somebody else since I'm not very experienced with jasmine and also someone can add to the docs(maybe a newcomer) very easily. What do you think? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#710 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ9Wf_d6nivbyt-uHbPk3ARwTtijHks5vTTqygaJpZM4aQOqP> .

harshkhandeparkar · 2019-03-04T15:32:18Z

Sure! On Mon 4 Mar, 2019, 9:01 PM Jeffrey Warren, <notifications@github.com> wrote:

…

Could you open up a new issue linking here and proposing adding tests built on this PR? We could then merge them together. Thanks! On Mon, Mar 4, 2019, 10:25 AM Harsh Khandeparkar ***@***.*** > wrote: > No need for an apology. Also I think tests and docs can be added in a > followup by somebody else since I'm not very experienced with jasmine and > also someone can add to the docs(maybe a newcomer) very easily. What do you > think? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > < #710 (comment) >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AABfJ9Wf_d6nivbyt-uHbPk3ARwTtijHks5vTTqygaJpZM4aQOqP > > . > — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#710 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AhKOn0Ru31HlfnplW5djSZ18GD82InJ4ks5vTTxMgaJpZM4aQOqP> .

harshkhandeparkar · 2019-03-04T15:32:37Z

Shall I open it now or after this is merged?

jywarren · 2019-03-04T16:32:21Z

Let's do it now, and wait to merge this until the test is completed. I prefer to have it be tested before merging, i hope you understand!

…

On Mon, Mar 4, 2019 at 10:33 AM Harsh Khandeparkar ***@***.***> wrote: Shall I open it now or after this is merged? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#710 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJzb87dlm7w0PWT7ndKCtKOzbNW2Oks5vTTyWgaJpZM4aQOqP> .

harshkhandeparkar · 2019-03-04T17:28:15Z

Yep. Sure!

examples/lib/intermediateHtmlStepUi.js

test/ui/defaultHtmlStepUi.spec.js

test/ui/mapHtmlTypes.spec.js

aashna27 · 2019-06-15T18:07:44Z

test/ui/scopeQuery.spec.js

+  })
+
+})
+


Okay so basically you are testing your new method by creating an example html! Why don't we test it for a particular step? I mean a module. I understand the results would be same but I want to know if you tried doing that and have been stuck as it happened with me as well!

It took me a day to figure this out.

Yeah the way is great as we basically have to test the functionality but have you tried it for a module? I just want to know if you faced issues with doing it with jasmine-jquery and fixtures and spyOnEvent

@aashna27 I didn't understand your question properly. If there is some problem in my code, please let me know here. If you want to ask some questions, DM me on gitter and resolve this conversation. Thanks!

I hear what @aashna27 is saying here, but although I see the logic in the suggestion of the HTML fixture being of similar structure to the one @harshkhandeparkar has used, I think it's basically OK for now. The important thing is that it serves a generalized purpose and that the HTML example here properly demonstrates it. Thanks!!!

Ok, now I understand. This is just a unit test, maybe we can write integration tests in the future. Currently, even the unit tests are incomplete so maybe integration tests and e2e tests can be written in the near future. What do ya'll say?

@aashna27 I didn't understand your question properly. If there is some problem in my code, please let me know here. If you want to ask some questions, DM me on gitter and resolve this conversation. Thanks!

No no problem with the code just wanted to know if you tried another approach, this one is great though!

examples/lib/intermediateHtmlStepUi.js

aashna27 · 2019-06-15T18:14:56Z

test/ui/scopeQuery.spec.js

+  })
+
+})
+


Yeah the way is great as we basically have to test the functionality but have you tried it for a module? I just want to know if you faced issues with doing it with jasmine-jquery and fixtures and spyOnEvent

jywarren · 2019-06-18T14:06:58Z

Hi, all, this looks really good and with one extra addition I think it is good to merge. @harshkhandeparkar you've written some great docs in the How does it work at top. And I see docs were added in #916 -- next time let's please merge docs and code in a single PR, even if we review it in multiple PRs - like, we could have the docs and tests and functions all merge into a joint feature branch maybe? Or just generally try to work in small pieces. But this looks really good and pending the "out of date branch" I see here being resolved, I think we are fine to merge it!

Also cc @MargaretAN9 who is working on an overall image-sequencer manual at https://publiclab.org/notes/MaggPi/06-10-2019/draft-of-public-lab-image-sequencer-user-manual-comments-welcome, as this will change some of the UI helper methods behaviors. @MargaretAN9 will you post your manual for comment in Markdown in a PR in this repository? Thank you!

harshkhandeparkar · 2019-06-18T14:10:20Z

@jywarren at the time the docs were written, I didn't think I would have time to write them myself so I made the FTO. But it ended up helping a first timer anyway 😄

jywarren · 2019-06-18T14:12:45Z

awesome!

…

On Tue, Jun 18, 2019 at 10:10 AM Harsh Khandeparkar < ***@***.***> wrote: @jywarren <https://github.com/jywarren> at the time the docs were written, I didn't think I would have time to write them myself so I made the FTO. But it ended up helping a first timer anyway 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#710?email_source=notifications&email_token=AAAF6J5B2ZR2HBM6PRL3VF3P3DUEZA5CNFSM4GSA5KH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODX6YBMI#issuecomment-503152817>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6J7IVQEFE5ODTXFZJJTP3DUEZANCNFSM4GSA5KHQ> .

outdated

harshkhandeparkar · 2019-06-18T14:15:11Z

Wow! I could rebase it from the browser. New GitHub features, which are added silently, are awesome.

examples/lib/intermediateHtmlStepUi.js

jywarren · 2019-06-18T19:12:26Z

All good here? Can you mention when it's totally ready to merge? Thank you!

examples/lib/defaultHtmlStepUi.js

test/ui/defaultHtmlStepUi.spec.js

Changes Needed

harshkhandeparkar · 2019-06-21T14:41:50Z

@aashna27 please review this again. Made the requested changes.

aashna27

Great, I think it is all set!! 🎉

harshkhandeparkar · 2019-06-21T17:25:07Z

Thanks a ton! @jywarren please review this.

…

On Fri, 21 Jun, 2019, 10:54 PM aashna27, ***@***.***> wrote: ***@***.**** approved this pull request. Great, I think it is all set!! 🎉 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#710?email_source=notifications&email_token=AIJI5H6XGZPB5JFX2JXAG5DP3UFD7A5CNFSM4GSA5KH2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB4JZK2I#pullrequestreview-252941673>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIJI5HZZFYDUUZLZ7VE322DP3UFD7ANCNFSM4GSA5KHQ> .

jywarren · 2019-06-21T19:11:14Z

🎉

harshkhandeparkar · 2019-06-22T13:27:25Z

Thanks a lot to everyone!
@jywarren @aashna27 @rexagod @publiclab/is-reviewers

tech4GT and others added 10 commits December 27, 2018 19:00

update dist

8e8281a

Signed-off-by: tech4GT <varun.gupta1798@gmail.com> dist update Revert "dist update" This reverts commit 9ee2a98.

Merge remote-tracking branch 'upstream/main' into main

4774f8f

Merge remote-tracking branch 'upstream/main' into main

305b45b

Merge remote-tracking branch 'upstream/main' into main

61a7c5d

Merge remote-tracking branch 'upstream/main' into main

d58a860

Merge remote-tracking branch 'upstream/main' into main

f7b42be

add dist

a51f89c

merge with main

f8acf7b

add new func

fe36581

update itermediate

7a57ad8

harshkhandeparkar requested review from Mridul97, rexagod, jywarren and a team January 24, 2019 06:38

HarshKhandeparkar added 2 commits January 25, 2019 11:32

Merge remote-tracking branch 'upstream/main' into step-selector

f2ae29e

changes

92c3bde

harshkhandeparkar added user-interface refactoring Issues/PRs which deal with code refactoring labels Mar 3, 2019

aashna27 reviewed Jun 15, 2019

View reviewed changes

examples/lib/intermediateHtmlStepUi.js Outdated Show resolved Hide resolved

aashna27 reviewed Jun 15, 2019

View reviewed changes

test/ui/defaultHtmlStepUi.spec.js Outdated Show resolved Hide resolved

aashna27 reviewed Jun 15, 2019

View reviewed changes

test/ui/mapHtmlTypes.spec.js Outdated Show resolved Hide resolved

aashna27 reviewed Jun 15, 2019

View reviewed changes

aashna27 requested changes Jun 15, 2019

View reviewed changes

jywarren approved these changes Jun 18, 2019

View reviewed changes

Merge branch 'main' into step-selector

2e97ffe

harshkhandeparkar commented Jun 18, 2019

View reviewed changes

harshkhandeparkar requested a review from aashna27 June 18, 2019 14:21

Remove double quotes

1513dce

aashna27 requested changes Jun 18, 2019

View reviewed changes

examples/lib/defaultHtmlStepUi.js Show resolved Hide resolved

aashna27 previously approved these changes Jun 18, 2019

View reviewed changes

test/ui/defaultHtmlStepUi.spec.js Outdated Show resolved Hide resolved

Harsh added 2 commits June 21, 2019 20:05

update new code

8e8ef36

refactor to spec

2bf17f9

harshkhandeparkar requested a review from aashna27 June 21, 2019 14:41

merge with mainn

9713720

aashna27 approved these changes Jun 21, 2019

View reviewed changes

Merge branch 'main' into step-selector

6e05607

jywarren merged commit 257113a into publiclab:main Jun 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shorten UI code with new `$step` method #710

Shorten UI code with new `$step` method #710

harshkhandeparkar commented Jan 24, 2019 •

edited

Loading

harshkhandeparkar commented Jan 24, 2019

harshkhandeparkar commented Jan 24, 2019 •

edited

Loading

harshkhandeparkar commented Jan 25, 2019

harshkhandeparkar commented Feb 28, 2019

jywarren commented Mar 4, 2019

jywarren commented Mar 4, 2019

harshkhandeparkar commented Mar 4, 2019

jywarren commented Mar 4, 2019 via email

harshkhandeparkar commented Mar 4, 2019 via email

harshkhandeparkar commented Mar 4, 2019

jywarren commented Mar 4, 2019 via email

harshkhandeparkar commented Mar 4, 2019

aashna27 Jun 15, 2019

harshkhandeparkar Jun 15, 2019

aashna27 Jun 15, 2019

harshkhandeparkar Jun 17, 2019

jywarren Jun 18, 2019

harshkhandeparkar Jun 18, 2019

aashna27 Jun 18, 2019

aashna27 Jun 15, 2019

jywarren commented Jun 18, 2019

harshkhandeparkar commented Jun 18, 2019

jywarren commented Jun 18, 2019 via email

harshkhandeparkar commented Jun 18, 2019

jywarren commented Jun 18, 2019

harshkhandeparkar commented Jun 21, 2019

aashna27 left a comment

harshkhandeparkar commented Jun 21, 2019 via email

jywarren commented Jun 21, 2019

harshkhandeparkar commented Jun 22, 2019

Shorten UI code with new $step method #710

Shorten UI code with new $step method #710

Conversation

harshkhandeparkar commented Jan 24, 2019 • edited Loading

How does it work

harshkhandeparkar commented Jan 24, 2019

harshkhandeparkar commented Jan 24, 2019 • edited Loading

harshkhandeparkar commented Jan 25, 2019

harshkhandeparkar commented Feb 28, 2019

jywarren commented Mar 4, 2019

jywarren commented Mar 4, 2019

harshkhandeparkar commented Mar 4, 2019

jywarren commented Mar 4, 2019 via email

harshkhandeparkar commented Mar 4, 2019 via email

harshkhandeparkar commented Mar 4, 2019

jywarren commented Mar 4, 2019 via email

harshkhandeparkar commented Mar 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jywarren commented Jun 18, 2019

harshkhandeparkar commented Jun 18, 2019

jywarren commented Jun 18, 2019 via email

harshkhandeparkar commented Jun 18, 2019

jywarren commented Jun 18, 2019

harshkhandeparkar commented Jun 21, 2019

aashna27 left a comment

Choose a reason for hiding this comment

harshkhandeparkar commented Jun 21, 2019 via email

jywarren commented Jun 21, 2019

harshkhandeparkar commented Jun 22, 2019

Shorten UI code with new `$step` method #710

Shorten UI code with new `$step` method #710

harshkhandeparkar commented Jan 24, 2019 •

edited

Loading

harshkhandeparkar commented Jan 24, 2019 •

edited

Loading