-
Notifications
You must be signed in to change notification settings - Fork 172
Game: Allow consistent, automatic switching between Game and Assessments #3105
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
Conversation
…r field instead of id
Pull Request Test Coverage Report for Build 14187283563Details
💛 - Coveralls |
…my/frontend into assessment-redirect-to-game
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.
There does not seem to be major issues with the PR. @JustATin555 Are you able to include screenshots of UI changes?
Also, once the PR is merged, we'll need to update the game dev guide and the storywriter guide for relevant portions on the id vs number.
Hmm... I wonder if the window might be a bit too different from the current assessment windows? (IIRC there is no window for submission on the workspace itself, but there is a window for submission from assessment list that looks a bit different) @RichDom2185 Any thoughts on standardizing styles for popup windows? I am ok with this for now since this will only be the case for the minigame, though I suspect that in the long run we want to minimize edge cases like this.. |
@lhw-1 the window is a modified copy of the assessment briefing card. Will likely refactor into a more standardised React component if time permits later on. |
My main concern is that assessments opened by the minigame are functioning differently from the regular assessments when I'm not sure if there is a need for that - can I check if the plan is to show these minigame assessments via the assessment list, or if they are only accessible through the game chapters? |
Should only be accessible through the game chapters - the category would have "display in dashboard" disabled. |
Alright, in that case we can go ahead with it. We can revisit the standardization at a later date as needed. |
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.
LGTM, thanks @JustATin555 for the PR! This is a nice update and should allow us to add back chapter-specific assessments, perhaps even in the form of minigames.
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.
LGTM, thanks!
Description
number
instead ofid
Type of change
How to test
Checklist