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

feat: add sample countdown trial that counts down before the start of… #496

Merged
merged 3 commits into from
Jun 26, 2024

Conversation

YUUU23
Copy link
Contributor

@YUUU23 YUUU23 commented Jun 19, 2024

  • add example countdown trial in trials folder inside honeycomb for counting down given millisecond (ms) before another trial begins:
    • in src/experiment/trials/countdown.js:
      • helper functions to convert ms to minutes and seconds to format a time string (00:00)
      • buildCountdownTrial function to return the actual JS trial object that counts down and updates the time string every few ms until counted down to 0
    • in src/experiment/honeycomb.js: an example usage of the countdown trial to countdown 6000ms and inserted in the experiment timeline

Copy link

github-actions bot commented Jun 19, 2024

Visit the preview URL for this PR (updated for commit 642f183):

https://ccv-honeycomb--pr496-add-countdown-trial-uqpt9ykm.web.app

(expires Wed, 03 Jul 2024 03:44:32 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 4ace1dcea913a952d2a1af84b94a4421bf36e610

@YUUU23 YUUU23 added the 4.0 Versioning: Issue in regards to version 4.0.0 release label Jun 19, 2024
@YUUU23 YUUU23 linked an issue Jun 19, 2024 that may be closed by this pull request
@YUUU23 YUUU23 added the good first issue Good for newcomers label Jun 19, 2024
@YUUU23 YUUU23 self-assigned this Jun 21, 2024
Copy link
Member

@RobertGemmaJr RobertGemmaJr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's chat about this a little closer on Tuesday? We have a special utils file in the lib/ folder that I think some of these functions would work perfectly! Love the way they're broken out and a little renaming would make them perfect for the entire project!

Fantastic work 🙌

Comment on lines 41 to 42
export function buildCountdownTrial(ms) {
const waitTime = ms;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename the parameter here to be waitTime instead of ms! You note that the time is in milliseconds in the docstring and that's the most common way time is handled in JS so you can be a little more explicit with the variable name of the function

/**
* Returns millisecond to minute
*
* @param {*} ms - millisecond
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the types here? It looks like they're almost always a number

Suggested change
* @param {*} ms - millisecond
* @param {number} ms - millisecond

@@ -47,8 +48,12 @@ export function buildHoneycombTimeline(jsPsych) {
// Builds the trials that make up the end procedure
const endProcedure = buildEndProcedure(jsPsych);

// Builds a countdown trial that counts down for 6000ms
const countdownTrial = buildCountdownTrial(6000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's make this 3000? Just so it's not taking up too much time

@YUUU23
Copy link
Contributor Author

YUUU23 commented Jun 24, 2024

@RobertGemmaJr Thank you for these feedbacks! I will go ahead and make those changes, and let's talk about it more on Tuesday's meeting! : )

… specific type in docstring, change time for countdown
Copy link
Contributor Author

@YUUU23 YUUU23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes discussed about should be changed!

@YUUU23 YUUU23 requested review from RobertGemmaJr and removed request for RobertGemmaJr June 26, 2024 03:49
Copy link
Member

@RobertGemmaJr RobertGemmaJr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonderful! Thank you for this!

Comment on lines 10 to 12
function getMinute(ms) {
return Math.floor(ms / 1000 / 60);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be moved into src/lib/utils!

Comment on lines 21 to 23
function getSeconds(ms, min) {
return Math.floor((ms - min * 1000 * 60) / 1000);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be moved into sec/lib/utils!

@RobertGemmaJr
Copy link
Member

@YUUU23 You can actually merge this pull request into 3.4.1! Sorry if that was confusing

@YUUU23 YUUU23 changed the base branch from feat-v4 to feat-v3.4.1 June 26, 2024 18:35
@YUUU23 YUUU23 added 3.4 Versioning: Issue in regards to version 3.4 release and removed 4.0 Versioning: Issue in regards to version 4.0.0 release labels Jun 26, 2024
@YUUU23 YUUU23 merged commit 890de74 into feat-v3.4.1 Jun 26, 2024
8 checks passed
@YUUU23 YUUU23 deleted the add-countdown-trial branch June 26, 2024 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.4 Versioning: Issue in regards to version 3.4 release good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Countdown example trial
2 participants