-
Notifications
You must be signed in to change notification settings - Fork 252
[Remove Vuetify from Studio] Alert dialog #5522
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
[Remove Vuetify from Studio] Alert dialog #5522
Conversation
MisRob
left a comment
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.
Welcome to the Studio codebase @kart-u. Nice first contribution. Just few cleanup notes, then should be good for merge.
...tcuration/contentcuration/frontend/channelEdit/views/files/thumbnails/ThumbnailGenerator.vue
Outdated
Show resolved
Hide resolved
...tcuration/contentcuration/frontend/channelEdit/views/files/thumbnails/ThumbnailGenerator.vue
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/shared/views/files/Uploader.vue
Show resolved
Hide resolved
|
Hi @MisRob, I’ve refactored the code as requested. Could you please review and confirm if it looks good? |
|
Thank you, @kart-u, it's on my review list - I will follow-up with you. |
|
The frontend test suite seems to be timing out. @kart-u It's possible the corresponding frontend tests need updated for the changes here, i.e. maybe |
|
@bjester yes I will look into this |
|
@bjester corrected test now working fine on my local tests |
MisRob
left a comment
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.
Thanks for taking care of the test too @kart-u @bjester. I don't quite understand why tests started timing out on this opportunity, but I read the test suite and the component - given the way it works now and what the test suite attempts to check, I think that the change is meaningful.
As for the alerts, I previewed them and confirm that they work as expected. Code changes make sense. Thanks!



Summary
Issues:-
Uploader.vue&ThumbnailGenerator.vueAlert.vueDescription of changes:-
Alertmodal toKModalin both filesAlert.vuefile:style="{ color: $themeTokens.annotation }"fortokens.annotationManual Testing Results
References
Closes #5472