-
Notifications
You must be signed in to change notification settings - Fork 231
Refactor 'unsaved changes' modal to use KModal component and update tests #5306
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
Refactor 'unsaved changes' modal to use KModal component and update tests #5306
Conversation
Thank you @Kartikayy007, we will assign a reviewer next week. |
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 just a few items that need to be cleaned up, but otherwise these changes are looking good, thank you @Kartikayy007!
import { set } from 'vue'; | ||
import { mapGetters, mapActions } from 'vuex'; | ||
import difference from 'lodash/difference'; | ||
import KModal from 'kolibri-design-system/lib/KModal'; |
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.
We don't have to import KModal
from KDS, this line can be removed :)
MessageDialog, | ||
ChannelItem, | ||
FullscreenModal, | ||
KModal, |
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.
Likewise, this line should be removed, we don't need to insert KModal
as a child component.
} | ||
}, | ||
confirmCancel() { | ||
this.showUnsavedDialog = false; |
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.
showUnsavedDialog
is set to false within this.close()
, which is called in this function already, so we don't need this line added here.
@Kartikayy007 are you planning to resolve feedback? |
@LianaHarris360 since we didn't hear from @Kartikayy007, noting that @vtushar06 may finish this work in this PR, so that you don't need to review everything again - as suggested here. |
…earningequality#5299). Addresses learningequality#5306 review feedback.
Closing in favor of #5432 |
…earningequality#5299). Addresses learningequality#5306 review feedback.
…earningequality#5299). Addresses learningequality#5306 review feedback.
Summary
Replaced Vuetify's
MessageDialog
component with Kolibri Design System'sKModal
for the unsaved changes dialog in the Collections modal.MessageDialog
withKModal
inChannelSetModal.vue
KModal
's API (title
,submitText
,cancelText
)Manual verification:
[email protected]
with passworda
KModal
stylingReferences
Reviewer Guidance
Test these changes by:
[email protected]
with passworda
Areas to focus on:
channelSetModal.spec.js
pass