-
Notifications
You must be signed in to change notification settings - Fork 880
Add confirmation modal to prevent accidental closure of Course Assignment Side Panel #14150
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
base: release-v0.19.x
Are you sure you want to change the base?
Add confirmation modal to prevent accidental closure of Course Assignment Side Panel #14150
Conversation
Build Artifacts
|
|
Hi @LianaHarris360, here are some issues I was able to identify while manually testing:
cannot.assign.course.mp4
no.changes.mp4The same should be valid if the user decides to click the 'Cancel' button if they haven't made any selections. If they have made any selection and they chose to click the Cancel button I think we should still show the confirmation modal.
|
| import { CoursesModals, PageNames } from '../../../../constants'; | ||
| import { overrideRoute } from '../../../../utils'; | ||
| import useAssignCourse from '../../composables/useAssignCourse'; | ||
| import CloseConfirmationGuard from '../../../../../../facility/frontend/views/users/common/CloseConfirmationGuard.vue'; |
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.
While we could reuse this, I think for the sake of wording/UX consistency within Coach (to some of Peter's comments), it might be easier to just use a KModal and repurpose the strings that we have in a few places.
There are definitely a few places where the logic is a little bit convoluted and more complicated 🙃 but, you can look here, as one example for reference.
The logic of the conditions for closing the side panel will also probably be a bit different, but looking at this may also give some ideas to address Peter's feedback around the checks, and confirming that some changes have been made.
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.
I made a new, similar component within kolibri/plugins/coach/frontend/views/courses/modals/, reusing how the previous CloseConfirmationGuard modal used ref and beforeRouteLeave because the side panel pushes to a different route to close it, but I fixed the console errors and updated the strings used to keep the same UX consistency in Coach, so the feedback has been handled now.
…osing without confirmation when selections are made
d608bbf to
868cf96
Compare
|
Hi @LianaHarris360, I confirm that the reported issues are fixed. Now there are just the following scenarios to be handled too:
2notshown.mp4
no.confirmation.needed.mp4 |

Summary
This pull requests adds a confirmation modal to
AssignCourseSidePanelso that while selecting and assigning a course, accidentally clicking outside of the panel won't close the side panel.closeconfirmationmodal.mp4
References
Closes #14140
Reviewer guidance