Watts-Lab / commonsense-platform

Commonsense platform
https://commonsense.seas.upenn.edu
1 stars 0 forks source link

111 removing hardcoded variable names #152

Closed acao22 closed 2 months ago

acao22 commented 3 months ago

Description: This pull request refactors MultiStepForm.tsx to eliminate hard-coded variables and use dynamic survey data from questions.ts, so that all the question and answer data is dynamically loaded & handled

Changes:

  1. Added some interface typing (IQuestionData) to be compatible with typescript (there are some errors with implicit any types)
  2. Replaced hard-coded variables like I_agree with questions.ts json
  3. Updated payload creation for Backend.post to use dynamic data

Thanks and let me know if there's anything I need to fix!

cypress[bot] commented 3 months ago

Passing run #68 ↗︎

0 11 0 0 Flakiness 0

Details:

Merge pull request #152 from Watts-Lab/111-removing-hardcoded-variable-names
Project: Commonsense Commit: 7e81805cb8
Status: Passed Duration: 00:44 💡
Started: Jul 9, 2024 5:30 PM Ended: Jul 9, 2024 5:31 PM

Review all test suite changes for PR #152 ↗︎

acao22 commented 2 months ago

I just resolved the merge conflict, should be good to go now. There is one review about connecting surveyData.json and answers.js together; I've fixed that but for some reason I can't resolve the comment. Thanks!

amirrr commented 2 months ago

I just resolved the merge conflict, should be good to go now. There is one review about connecting surveyData.json and answers.js together; I've fixed that but for some reason I can't resolve the comment. Thanks!

No problem this is good enough. just check to make sure this works on your local system and then merge

amirrr commented 2 months ago

You can fix the package-lock.json by running npm i --package-lock-only

acao22 commented 2 months ago

@amirrr , is it possible for you to help me close the requested change? I don't see where I can indicate it is resolved but I combined surveyData.json and answers.js so it should be fixed, I can't merge without resolving that change. Thanks!