-
-
Notifications
You must be signed in to change notification settings - Fork 681
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modified Visual Search Circle Plugin #1954
base: main
Are you sure you want to change the base?
Conversation
Thanks @keshavpabbi ! Would you be willing to modify the corresponding pages in |
|
Sure! Is there anything specific I should modify in the docs folder? Also I closed this by accident. Do I have to submit another pull request? |
Hi @keshavpabbi, I just re-opened this 😃 I think the documentation pages that need editing are:
I had a quick look at your changes to the plugin file and didn't notice any other changes to how the plugin works. But if you made any other changes then please mention those on the docs page as well. Another thing to note is that we're currently making changes to the Examples sections in the documentation pages anyway, which you can read about here: #1923 and here: #164. So if you want to incorporate these changes to the visual search documentation page too, then that would be great, but no problem if not! Thanks!! |
changed the name of visual-search-circle to visual-search and changed the description to show new code added to the plugin
renamed visual-search-circle to visual-search, changed description for the plugin
Hi @keshavpabbi, thanks very much for editing the documentation pages! I'll just test the way your updated version of this plugin works. If I have any questions or issues, I'll let you know, otherwise this looks ready to merge 👍 |
And don't worry about the failing Jest test. If you want to, then feel free to fix whatever test is failing and include any plugin or test file modifications in this PR. But if you don't want to or aren't sure how then I can do it. |
Ah, I just checked the Jest test errors and it looks like an easy fix: the old plugin name is used in the file "tests/plugins/plugin-visual-search-circle.test.js". So I'm guessing the tests will pass when this is changed. Again, feel free to change it and include it in this PR, otherwise I can do it 😃 |
So just to confirm, am I supposed to change the name of the js file? |
Sorry for not being clear. The name of the file does need to be changed, but the error is actually caused by references to the old plugin name inside of that file. Let me know if you have any other questions or if you'd like me to help out with these final changes. Thank you! |
…l-search.test.js replaced "visual-search-circle" with "visual-search"
Alright so I changed the name of the file and wherever it said "visual-search-circle" in the code, I replaced it with "visual-search". Is there anything else I should fix? |
Yep that's perfect, thanks! Note that this is marked for the 7.1 release. Right now we're working some other changes for the next release (7.0), and then we'll switch to 7.1, which will be focused on adding and improving plugins. So this pull request will be merged sometime after we've finished 7.0, which I'm guessing will be in about a month. |
Sounds good Becky! |
Hi @keshavpabbi, We created a new community contributions repository that would be a great immediate home for this plugin. We'll definitely keep this on the agenda for merging in the long term, but we have some other priorities so it'll be a few months at least. |
I have replaced the visual-search-circle plugin with a modified version of the plugin called visual-search. This plugin allows experimenters to run a visual search experiment with images placed on a grid if they choose not to use the circle format. I have also added a jitter parameter that can be used by experimenters.
(jitter_ratio and usegrid are the 2 parameters I have added to the visual-search plugin)