-
Notifications
You must be signed in to change notification settings - Fork 273
[Remove Vuetify from Studio] Content Library catalog page (#5526) #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] Content Library catalog page (#5526) #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
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.
The test suite looks nice overall, thank you.
After #5536 is in unstable, I will
- Merge
unstabletochannel-cards - Then you can merge latest
channel-cardsto your localchannel-cards - And resolve conflicts if any
Sorry for that - I know that delayed review time contributed to this. I will be ready to support.
But for now, let's wait until we merge #5536.
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.
No worries about the delayed review time, I totally understand things come up.
I'll be ready to proceed as soon as #5536 merges
| <Checkbox | ||
| <KCheckbox | ||
| v-else-if="selecting" | ||
| v-model="selectAll" |
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.
At this place, we can keep v-model - it will be simpler to maintain.
I think that when you implemented this PR, KCheckbox didn't yet support v-model, but we recently implemented it - you can see the latest documentation. You will need to merge latest unstable to this branch and run pnpm install so that you have access to the latest KDS version.
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.
Oh sorry, when I was writing this I forgot for a moment that we're targeting channel-cards. So actually let's wait a bit for this update until I do this and then you can resolve it together with test conflicts.
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.
Understood, I'll wait for #5536 to merge to unstable, then merge latest channel-cards, update dependencies with pnpm install,and apply the v-model update. Ready to proceed after that.
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