Skip to content

Add question number to ControlBar #366

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

Merged
merged 3 commits into from
Sep 11, 2018
Merged

Add question number to ControlBar #366

merged 3 commits into from
Sep 11, 2018

Conversation

jiachen247
Copy link
Contributor

Previously, when students do the missions/quizzes we have no clue how many questions the mission contains.

This makes it much easier to track progress when doing any kind of assessment.

image

@jiachen247 jiachen247 changed the title added assessment status ("Question 1 of 1") to control bar in workspace. added assessment status to control bar in workspace Sep 8, 2018
@coveralls
Copy link

coveralls commented Sep 8, 2018

Pull Request Test Coverage Report for Build 848

  • 1 of 8 (12.5%) changed or added relevant lines in 2 files are covered.
  • 65 unchanged lines in 5 files lost coverage.
  • Overall coverage increased (+0.05%) to 25.849%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/components/workspace/ControlBar.tsx 0 7 0.0%
Files with Coverage Reduction New Missed Lines %
src/mocks/gradingAPI.ts 6 23.08%
src/components/academy/grading/index.tsx 14 25.81%
src/components/assessment/index.tsx 14 81.2%
src/components/assessment/AssessmentWorkspace.tsx 14 60.44%
src/sagas/backend.ts 17 4.78%
Totals Coverage Status
Change from base Build 821: 0.05%
Covered Lines: 903
Relevant Lines: 2959

💛 - Coveralls

@remo5000
Copy link
Contributor

remo5000 commented Sep 9, 2018

@jiachen247 Thanks for the initiative! This looks good so far, but I feel that with the inclusion of question numbers, the logic behind having previous/next/return buttons can be shifted to the ControlBar. This means that AssessmentWorkspace and GradingWorkspace only need to pass the question number and max question number, and the previous/next button logic is taken care of using the question number information inside ControlBarProps. Could you implement this for this PR? Do let me know if you have any doubts

@remo5000 remo5000 changed the title added assessment status to control bar in workspace Add question number to ControlBar Sep 9, 2018
@jiachen247
Copy link
Contributor Author

jiachen247 commented Sep 9, 2018

@remo5000 yepps!!! totally agreee - cleaner this way :-)

have shifted responsibility of flow control entirely into the control bar.

hope it helps!!

Copy link
Contributor

@remo5000 remo5000 left a comment

Choose a reason for hiding this comment

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

LGTM

@remo5000
Copy link
Contributor

@jiachen247 Whoops, it seems like you pushed without formatting and updating the tests -- could you run yarn format and yarn test to resolve these? I'll merge it after

@jiachen247
Copy link
Contributor Author

@remo5000 great! have formatted the code and have updated the failing snapshots :-)

@remo5000 remo5000 merged commit 3eb0e9e into source-academy:master Sep 11, 2018
@remo5000
Copy link
Contributor

@jiachen247 Merged, cheers!

@jiachen247
Copy link
Contributor Author

@remo5000 when will the changes go live? haha

@remo5000 remo5000 mentioned this pull request Sep 12, 2018
@remo5000
Copy link
Contributor

@jiachen247 We're going to be deploying v1.0.9 soon 🤞

@jiachen247
Copy link
Contributor Author

@remo5000 sounds good man! looking forward to it. thanks for all the hard work!! its an amazing platform :-)

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.

3 participants