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

finished Stroop Color Word #10

Merged
merged 6 commits into from
Mar 12, 2021
Merged

finished Stroop Color Word #10

merged 6 commits into from
Mar 12, 2021

Conversation

steffejr
Copy link
Contributor

@steffejr steffejr commented Mar 9, 2021

Here is a the Stroop Color, Word and Color/Word tasks.

@pjkohler
Copy link
Collaborator

pjkohler commented Mar 9, 2021

Looks great!

  • are each of the 3 html files meant to be run as separate experiments? I could not get StroopColor.html to run.
  • if so, could you specify in the Readme what happens in each experiment?
  • you may want to change the title in the header of each experiment (still says "NCM lab")
  • you may want to comment out jatos if you are not using it

@pjkohler
Copy link
Collaborator

pjkohler commented Mar 9, 2021

Could you make full-screen mode an optional parameter?

@steffejr
Copy link
Contributor Author

steffejr commented Mar 9, 2021 via email

@pjkohler
Copy link
Collaborator

pjkohler commented Mar 10, 2021

apologies, @steffejr, I don't think we want to mess with override_safe_mode after all, as pointed out here: #6 (comment)

Could you take that out again? Everything looks good, but I will try to run through the experiments tomorrow for a final check.

@pjkohler
Copy link
Collaborator

I ran through all 3 experiments and they look GREAT. I think we are about ready to merge this pull request.

I have made some minor changes so that Correct and correct are not both in the data file and practice and experiment trials are labelled differently. I think you need to make me a contributor before I can push them. @jodeleeuw is that the standard practice when a maintainer makes edit to a PR?

@steffejr
Copy link
Contributor Author

Thank you Peter. I want to see how you labeled the trials differently. I could not figure that out.
I do not see how to add you as a contributor to my fork.

I wonder is better for a task like this: to have one folder containing the three related versions of the task; or to have three folders?

The thing is, you really would not do one without the other two.

@pjkohler
Copy link
Collaborator

Can you not do it under settings -> manage access -> invite a collaborator?

@pjkohler
Copy link
Collaborator

I think 3 experiments are fine for now. Eventually I would want to have a single experiment, where the settings determine which subset of color block, word block and color-word block to run.

My changes are ready when you give me permission.

@steffejr
Copy link
Contributor Author

steffejr commented Mar 11, 2021 via email

@pjkohler
Copy link
Collaborator

Okay, pushed my changes. Two more minor things:

  • why is the thank you msg different for color-word compared to the other two?
  • it might be good to think about some other way to change the background colour. I tried putting the experiment on cognition.run, and there is no way on there to include style settings like you use for the background. I am not that familiar with cognition.run, @jodeleeuw might know more.

For now I think you can just check my changes (everything should work), standardize the thank you message and then we can merge.

@pjkohler
Copy link
Collaborator

pjkohler commented Mar 11, 2021

okay, I moved background color settings into a css file with the same name as the experiment folder. I think we might make that standard practice. The goal is to make adding the experiment to cognition.run as easy as possible.

can you move the viewport settings (line 9) into the css file as well, or is that not possible?

@steffejr
Copy link
Contributor Author

I just tested it and it works great. I like what you did Peter for tagging the trials. I never would have thought of that.

I think it is ready to merge.

@pjkohler pjkohler merged commit 3365f43 into jspsych:main Mar 12, 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