-
Notifications
You must be signed in to change notification settings - Fork 4
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
Building nback #23
Building nback #23
Conversation
🦋 Changeset detectedLatest commit: 14095fe The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks pretty good overall, just adding a couple of pointers and things to clean up!
packages/n-back/dist/index.global.js
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure why but the dist files are still in the repo- make sure to remove them and try running npm run build
again, if they show up again, there may be a problem in the root .gitignore
file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the .gitignore file included that. Checking what's wrong
packages/n-back/README.md
Outdated
|
||
## Compatibility | ||
|
||
`@jspsych-timelines/n-back` requires jsPsych v7.0.0 or later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it be possible to turn this timeline into supporting v8 immediately?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I didn't use any functions that were affected by the change from v7 to v8, so I'll probably just update the README
packages/n-back/src/index.ts
Outdated
choices: "NO_KEYS", | ||
on_start: function () { | ||
if (data_output == "csv"){ | ||
jsPsych.data.get().localSave('csv', `n_back.csv`);} else if (data_output == "json") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just wondering, is it normal to .localSave()
, or can we give the developer the option to locally save or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented the option
No description provided.