Skip to content
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

Ebbinghaus Experiment #14

Merged
merged 7 commits into from
Sep 16, 2021
Merged

Ebbinghaus Experiment #14

merged 7 commits into from
Sep 16, 2021

Conversation

pjkohler
Copy link
Collaborator

Pull request for my version of the Ebbinghaus Experiment. Pretty simple, should be pretty much ready to go. Put happy to hear about what I may have overlooked.

@pjkohler pjkohler requested a review from jodeleeuw March 13, 2021 21:10
@pjkohler pjkohler self-assigned this Mar 13, 2021
@pjkohler pjkohler mentioned this pull request Mar 16, 2021
@pjkohler
Copy link
Collaborator Author

@jodeleeuw: Could you review this at some point?

@steffejr
Copy link
Contributor

I tested this and it works great. Here are a few comments.

The instructions after every trial is super distracting. Can you put the instructions before each block? Otherwise it is a lot of flashing on the screen. Or you can keep the instructions on the screen continuously?

If we are going to use setup files with specific languages, then all text that the participant sees should be included in the setup file and called as variables in the experiment. This makes it easier to modify and for us to provide experiments in multiple languages where only the setup file changes.

The code looks very neat and clean and the readme and setup files look great.

Nice job

@pjkohler
Copy link
Collaborator Author

good suggestions - will address them shortly.

/Peter

@pjkohler
Copy link
Collaborator Author

Alright, I think I addressed your concerns. The issue with the messages to participants is that they utilize variables that are declared when the code is run. I found a workaround using string replacement, but I would be interested if anyone knows of a cleaner way to do it. Take a look.

@steffejr
Copy link
Contributor

There is a problem with the ebbinghaus_setup_EN.js file on line #22 in the

. I don't know what the problem is though.
Other than that it looks good.

@pjkohler
Copy link
Collaborator Author

weird, I don't know how I was ever able to run with that code. I added the text color change (to white) to set css file, which is probably better anyway. Merging.

@pjkohler pjkohler merged commit f051388 into jspsych:main Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants