-
Notifications
You must be signed in to change notification settings - Fork 281
[Remove Vuetify from Studio] Cards in Content Library #5616
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: channel-cards
Are you sure you want to change the base?
[Remove Vuetify from Studio] Cards in Content Library #5616
Conversation
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
|
Season's greetings! 👋 We'd like to thank everyone for another year of fruitful collaborations, engaging discussions, and for the continued support of our work. Learning Equality will be on holidays from December 22 to January 5. We look forward to much more in the new year and wish you a very happy holiday season! Are you preparing for Google Summer of Code? See our GSoC guidelines. |
|
👋 Thanks for contributing! We will assign a reviewer within the next two weeks. In the meantime, please ensure that:
We'll be in touch! 😊 |
|
@MisRob I've implemented using KCardGrid with the card's
|
|
Hi @vtushar06, thanks! As mentioned elsewhere, I will get back to this work after I am back from vacation later in January. As for your question - thanks for raising that. Please use the standard KDS pattern Also, I noticed that you added a completely new page |
|
Hi @MisRob, thank you for the feedback, I understand you'd like a more minimal approach for this PR. I followed the pattern from the reference PRs (#5227, #5524, #5525), which all created new complete page components (StudioMyChannels, StudioStarredChannels, StudioViewOnlyChannels). Since the guidance in #5526 said to "replace" the section and "use KCardGrid", I created StudioCatalogList.vue as a complete KDS implementation. If you prefer a hybrid approach instead, I will make changes accordingly. |
|
@vtushar06, this issue is a bit different from others for a number of reasons - I think you can just follow the issue's guidance and it should be fine. |
|
Yeah @MisRob now I applied changes according to guidance, I got so much into other issue learning that's why I used similar approach. |
|
Hii @MisRob, this one is also ready for your review, although I have gone once more in changes so there may not be any issue in review process. |
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.
Thank you @vtushar06, overall looks very good! Only one clarification and one suggestion.
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogList.vue
Show resolved
Hide resolved
...entcuration/contentcuration/frontend/channelList/views/Channel/__tests__/catalogList.spec.js
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogList.vue
Outdated
Show resolved
Hide resolved
|
Hi @vtushar06,
Then you can run Thank a lot. |
c5ceb61 to
0cc3d1d
Compare
|
Hi @MisRob, |
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 @vtushar06! Let's have a look at few remaining areas.
...curation/contentcuration/frontend/channelList/views/Channel/components/StudioChannelCard.vue
Outdated
Show resolved
Hide resolved
...curation/contentcuration/frontend/channelList/views/Channel/components/StudioChannelCard.vue
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogList.vue
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogList.vue
Outdated
Show resolved
Hide resolved
contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogList.vue
Show resolved
Hide resolved
|
You were right.I initially removed detailsRouteName thinking it was unused, but after checking the actual behavior, I realized that it is useful. becoz When I removed, clicking a catalog card navigated to the full edit page instead of showing a read-only details modal. |
|
heyy @MisRob, can you please look into recents changes as per your review. |
Summary
Removes Vuetify from Content Library catalog page and implements with KDS components.
Changes:
StudioCatalogList.vueusing KDS components (KCardGrid, KCheckbox, KButton)StudioChannelCard.vueusing KCard#selectslotScreenRecording:
Studio-5526.mp4
Studio-5526-2.mp4
…
References
Fixes #5526
Related PRs
…
Reviewer guidance
…
Verify Tests:
/channels/catalog)Run tests
pnpm run test -- catalogList.spec.js