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

Large Package Refactoring #9

Open
wants to merge 4 commits into
base: package-refactor
Choose a base branch
from

Conversation

Willy-Chan
Copy link
Collaborator

  • Deleted assets, assets are now loaded in via google cloud buckets (using the respective URI)
  • Restructured the timeline flow, so now it goes through the RoarMP class located in index.js. This is eventually run (with the buildexperiment function from experiment.js) in conjunction with serve.js for the final timeline.
  • Noted Error: Unable to set the language setting: are we still doing this through URL queries?

NOTES:

  • I'm unsure as to whether or not the trial data is saving to the correct location, or even saving at all (I'm unable to locate the firebase entry for roar-mp)
  • Infrastructure exists for multiple language settings, but still have yet to be implemented depending on how the new refactored version should handle that.

@Willy-Chan Willy-Chan requested review from richford and Zio-4 August 10, 2023 21:50
@Zio-4
Copy link

Zio-4 commented Aug 11, 2023

Hey Willy, looks good overall. Going to add some comments on some changes to make.

assets.json Show resolved Hide resolved
cors-config.json Outdated Show resolved Hide resolved
serve/serve.js Show resolved Hide resolved
serve/serve.js Outdated Show resolved Hide resolved
src/experiment/config/config.js Outdated Show resolved Hide resolved
src/experiment/trials/introduction.js Outdated Show resolved Hide resolved
src/experiment/trials/introduction.js Outdated Show resolved Hide resolved
src/locales/en/translation.json Show resolved Hide resolved
assets.json Outdated Show resolved Hide resolved
src/experiment/i18n.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
src/experiment/experiment.js Outdated Show resolved Hide resolved
… timeline now uses the function from config.js
@Willy-Chan Willy-Chan requested a review from AnyaWMa October 2, 2023 17:25
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