8110/mark resource as finished modal part deux #8403
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nucleogenesis
added
TODO: needs review
nucleogenesis
force-pushed
the
8110/mark-resource-as-finished-modal-part-deux
branch
from
ab8a2dc to
1fc1d91
Compare
nucleogenesis
force-pushed
the
8110/mark-resource-as-finished-modal-part-deux
branch
from
1fc1d91 to
225b871
Compare
marcellamaki
previously requested changes
Sep 13, 2021
There was a problem hiding this comment.
Hi @nucleogenesis - this looks good and the completion is working as expected (state updating when confirmed, not on cancel). The only problem that I see is that the modal doesn't close after confirming automatically. (It requires clicking "cancel" after clicking "confirm" at the moment, which is not ideal)
Probably a small fix, and with this updated, should be good to merge!
nucleogenesis
force-pushed
the
8110/mark-resource-as-finished-modal-part-deux
branch
from
225b871 to
47dc9b8
Compare
- LearningActivityBar is hard coded to show when not in production - should be from content metadata. - Added GlobalSnackbar to LearnImmersiveLayout - MarkAsComplete modal now uses available Vuex action to dispatch progress update.
nucleogenesis
force-pushed
the
8110/mark-resource-as-finished-modal-part-deux
branch
from
47dc9b8 to
f4c6252
Compare
rtibbles
approved these changes
Sep 14, 2021
There was a problem hiding this comment.
Typo causing the issue that @marcellamaki reported has been fixed. Merging! We can find any other issues after full implementation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Now the pop-up for Mark As Complete works when the context menu item is clicked.
Note: There is currently no indicator of resource status on the immersive view so you must go back to the topic page to see that the completion occurred.
Note: Whether the menu option is available is currently determined by whether you're
process.env.NODE_ENV === 'production'or not. So - devs will see it, nobody else will until we hook in the correct content metadata.References
Fixes #8110
Reviewer guidance
Go to resources you've not completed. Find the "Mark as Complete" menu item top-right context menu when viewing the resource.
Test canceling and submitting - make sure that the outcome you expect occurred by going back to the topic where you found the resource.
Testing checklist
PR process
Reviewer checklist
yarnandpip)