plugin-meye-config & extension-meye #73
Conversation
🦋 Changeset detectedLatest commit: 688269b The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hi @AdamVasarhelyi2, Thanks for the contribution! It looks like there are files in |
|
Sure Josh! Thanks for the feedback - I'm a little new :P I'll get this done by Monday |
Hey Josh sorry for taking forever, I'm in a super hectic part of my last semester at college. I'll get around to this soon :) |
|
Hey @jodeleeuw, I've finally gotten around to this. Sorry for taking so long!! I hope everything's okay now - I (think I) have removed the files from the /dist directory that were causing problems with the first submission. Please let me know if everything is okay! |
|
@AdamVasarhelyi2, sorry for letting this slip through the cracks! I've updated a few things to match existing conventions. The next step is getting the package-lock.json to update, but I don't have permission to push directly to your fork for that. Can you pull my changes and then run |
|
Thanks a lot, Josh! Means a lot to me that we can get this working 😄
I also happened to check this email account by accident and am much more reliable on ***@***.***
Will get to work!
Adam
…________________________________
From: Josh de Leeuw ***@***.***>
Sent: Wednesday, July 10, 2024 12:21 AM
To: jspsych/jspsych-contrib ***@***.***>
Cc: Adam Vasarhelyi ***@***.***>; Mention ***@***.***>
Subject: Re: [jspsych/jspsych-contrib] plugin-meye-config & extension-meye (PR #73)
@AdamVasarhelyi2<https://github.com/AdamVasarhelyi2>, sorry for letting this slip through the cracks! I've updated a few things to match existing conventions. The next step is getting the package-lock.json to update, but I don't have permission to push directly to your fork for that. Can you pull my changes and then run npm i in the root of the repository to generate an updated package-lock.json file? When you push that I think the build should work.
—
Reply to this email directly, view it on GitHub<#73 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHOQRSZJGMBSCNSA4S5TJGDZLPWWFAVCNFSM6AAAAABKS4XTP6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJXHA3TCNZZGA>.
You are receiving this because you were mentioned.
|
i am new to git and I really hope this works :C
|
Hey Josh! All done 🙂 It's kind of fun learning git!
Let me know if you need me to do anything else. Thanks.
Adam
…________________________________
From: Josh de Leeuw ***@***.***>
Sent: Wednesday, July 10, 2024 12:21 AM
To: jspsych/jspsych-contrib ***@***.***>
Cc: Adam Vasarhelyi ***@***.***>; Mention ***@***.***>
Subject: Re: [jspsych/jspsych-contrib] plugin-meye-config & extension-meye (PR #73)
@AdamVasarhelyi2<https://github.com/AdamVasarhelyi2>, sorry for letting this slip through the cracks! I've updated a few things to match existing conventions. The next step is getting the package-lock.json to update, but I don't have permission to push directly to your fork for that. Can you pull my changes and then run npm i in the root of the repository to generate an updated package-lock.json file? When you push that I think the build should work.
—
Reply to this email directly, view it on GitHub<#73 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AHOQRSZJGMBSCNSA4S5TJGDZLPWWFAVCNFSM6AAAAABKS4XTP6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJXHA3TCNZZGA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
There was a problem hiding this comment.
hey, this plugin and extension looks like a fantastic idea and we'd love to include it. i've added some code review notes for you to view at your leisure, but also just one important distinction is that i haven't actually been able to run the code, no matter what I do, apparently my browser just isn't powerful enough, but i will continue to try and run it later on and let you know, these are just some helpful tips for you. thanks again for the contributions!
| introduction(); | ||
| }; | ||
|
|
||
| function introduction() { |
There was a problem hiding this comment.
i'm wondering if it's just worth leaving this out and instead having a user just use an html-button-response or instructions plugin beforehand: it would also give them more power to customize this prompt
There was a problem hiding this comment.
I really wanted it to be part of the plugin because they're special instructions that are unique to the plugin and are unlikely to change or be different between research. The reason it's unlikely to change is because orthogonal lighting is better for it and outdoor lighting is orthogonal. I can still change this if you prefer, though.
There was a problem hiding this comment.
as i mentioned above with parameters, probably the place that language would come in would be if we were to serve this experiment to someone that doesn't speak english. maybe the special instructions are unlikely to change between different experiments, but it'll make it harder to serve the experiment to those that speak other languages
There was a problem hiding this comment.
I see, all good. Would you prefer that I remove that part of the instructions or are you suggesting that I add a parameter for localisation? The former option wouldn't fix the fact that in the actual calibration part of the plugin, there are instructions in English.
There was a problem hiding this comment.
probably a parameter for localization, just something along the lines of being able to change the instructions that you start the plugin out with so that others can provide their own translations
|
Hey @jadeddelta I've pushed the updates for the plugin and extension that you've requested. I'm still learning Git so I hope it worked! As for the issue of why it didn't seem to work at all for you (something about your browser not being powerful enough), I have some ideas.
Let me know if these work! Thanks, |
|
hii @AdamVasarhelyi2, so sorry for the delay, i'm stuck on break without my camera so i can't exactly test the plugin at the moment, and i get back at around the 21st so that'll be where I can review this PR. |
|
No wories @jadeddelta :) Enjoy your holiday! |
|
@jadeddelta Just following this up - Have you had a chance to test it again? Kind regards, |
There was a problem hiding this comment.
was able to check a little bit more on this pass, some minor issues in the extension that needs to be addressed. As for the plugin-meye-config, i noticed that if the screen is small enough, the <aside> info-box element has too much information and cuts off vertically, making it unable to access the buttons without scrolling down, so either making it able to scroll or consolidating information elsewhere would be great
…odel url parameter
|
Hey @jadeddelta , I made some major improvements to just about everything. It runs more efficiently with fewer bugs, much cleaner code, and I have implemented your feedback. Please let me know if there is anything else :) Getting very excited to release this! |
|
hey @AdamVasarhelyi2, i think we're getting very close to a release! i think the only major thing that i'm particularly worried about, especially looking at it now, is the viewbox again. i'm wondering if it's possible to consolidate all of the information such that the participant doesn't have to scroll, especially since the calibrate button requires one to scroll down to see it- maybe via putting the calibrate button directly under the big camera preview, or shortening the instructions/lowering the font size- whatever works! as for what i did- i just fixed a ts bug that cropped up while reviewing- and i also merged and added the changeset so that everything is basically ready to be merged when needed. |
|
Hey Jade, thanks. I've been super busy but letting you know I'm getting around to this! |
Presenting a new plugin (plugin-meye-config) and extension (extension-meye) for review. Steps to verify functionality:
In cmd.exe:
Run
npm installin the root directoryRun
npm run buildinside packages/plugin-meye-config and packages/extension-meyeNavigate to /packages (i.e., go up a directory)
Run a local python server with
python -m http.serverIn your web browser, navigate to http://localhost:8000/
To test the plugin, navigate to /plugin-meye-config/examples. To test the extension, navigate to /extension-meye/examples. Note that the extension example calls the index.browser.min.js file from the plugin's /dist directory as the plugin and extension are to be used together.
Once the examples finish, they output data that you can examine.
The original website for the technology implemented by the plugin / extension is https://www.pupillometry.it/
In both the original mEye and the plugin, it is difficult for calibration to be perfect (it is harder in the original website because calibration must be done manually, but has been automatized in the plugin). If testing is difficult due to lighting constraints, you can print out a picture of an eye and hold it up to the camera. If you put your hand over the pupil, the extension's data for blink probability should increase to 1.
Please let me know if you have any questions :)