-
Notifications
You must be signed in to change notification settings - Fork 225
[Remove Vuetify from Studio] Collection channels loader in Channels - New collection #5388
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: unstable
Are you sure you want to change the base?
[Remove Vuetify from Studio] Collection channels loader in Channels - New collection #5388
Conversation
This reverts commit 0701d5b.
…earningequality#5244)" This reverts commit f9bee66.
…earningequality#5244)" This reverts commit 96f21da.
Thank you @nishatalam24, we will assign a reviewer soon. |
.loader-text { | ||
margin-top: 16px; | ||
font-size: 1.2em; | ||
color: #555555; |
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.
Lets use the the KDS colors here. There are quite a number of examples on how this can be done eg $themeTokens
, $themePalette
, etc. Heres a example
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.
Because of https://github.com/learningequality/studio/pull/5388/files#r2388418039, this css class should be removed as well
<KCircularLoader :size="70" /> | ||
<div class="loader-text"> | ||
<slot ></slot> | ||
</div> |
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.
This is not required, so should be removed.
align-items: center; | ||
justify-content: center; | ||
min-height: 180px; | ||
} |
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.
Simplify this css class in light of https://github.com/learningequality/studio/pull/5388/files#r2388418039
flex-direction: column; | ||
align-items: center; | ||
justify-content: center; | ||
min-height: 180px; |
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 think the loader size should be set using the size
prop of the KCircularLoader
so setting min-hight
may not be useful in this case.
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.
Hi @nishatalam24! Great work so far! We have the building blocks in place, so just a little clean up and we should be good to go! Thanks!
Also a reminder to take a look at the linting errors reported here. I suspect that you haven't install the pre-commit hook. To install the hook, run |
Hi @nishatalam24! This is just follow up about the progress on this. Hope everything is okay? |
Summary
This pull request addresses and closes issue #5244 by creating a new
StudioLargeLoader
component and replacing the old loaders in two locations.Changes
StudioLargeLoader.vue
, was created to be a reusable, Vuetify-free loader that uses the Kolibri Design System's<KCircularLoader>
.ChannelSetModal
and theStorage
overview page.References
This pull request closes #5244.
Reviewer Guidance
All user interactions have been manually tested, and the loaders now function as expected with a modern, KDS-compliant design.