Channel cards to unstable by MisRob · Pull Request #5696 · learningequality/studio

MisRob · 2026-02-10T15:26:50Z

Summary

Replaces Vuetify-based implementation by KCard on My Channels, Starred Channels, View-only Channels, and Content Library pages. On this opportunity introduces a new architecture of the related pages and resolves problems with understanding & maintaining monolithic and condition-heavy ChannelList + ChannelItem.

Before	After

References

Closes #5227
Closes #5524
Closes #5525
Closes #5526

Reviewer guidance

Code review:

80% of work are community contributions that were reviewed by myself and some also by @rtibbles. Then I added a final layer of work: abstractions, cleanup, user experience as well as internal architecture optimizations, fixes for few regressions, ... => I think it'd most meaningful to review affected files as a whole.

Local preview:

Run pnpm install first
Preview My Channels, Starred Channels, and View-only Channels pages
Preview Content Library when logged in and logged out

Notes

Before merging, [TODO REVERT] Temporarily install KDS fork commit needs to be reverted and a newly released KDS version with learningequality/kolibri-design-system#1203 installed.

Unstable to channel cards branch

KDS has recently started providing visuallyhidden so is not needed.

and related re-organization of files and tests. Also adds more tests to some components.

shape of loaded cards.

bjester

Some review comments

contentcuration/contentcuration/frontend/channelList/composables/useChannelList.js

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioChannelCard/index.vue

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioMyChannels/index.vue

MisRob · 2026-02-18T07:30:55Z

~~I've just noticed on screenshots that I lost the invitations box being a bit wider than cards. Will fix this ~ is good for visual priority.~~

Resolved.

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioChannelsPage/index.vue

contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogList.vue

akolson

I think code is clear and understandable. Thanks Misha for this refactor!

to prevent unnecessary memory usage and to improve architecture. Relatedly remove props for configuring footer in favor of the footer slot to make communication between the modals and footer buttons straightforward.

in channel token modal and remove unnecessary token tracking handlers from components (StudioCopyToken handles tracking)

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioMyChannels/index.vue

rtibblesbot

Well-structured PR that replaces Vuetify-based channel cards with KDS KCard across four pages, with a clean architectural decomposition from monolithic ChannelList+ChannelItem into focused per-page components.

CI passing. Screenshots verified for My Channels, Starred, View-only, and Content Library — all look visually consistent and complete.

Suggestions:

Missing notranslate class and dir="auto" on channel description in StudioChannelCard (see inline comment)
Missing error handling in useChannelList composable — rejected dispatch leaves loading state stuck (see inline comment)

Praise:

Excellent decomposition into StudioMyChannels, StudioStarredChannels, StudioViewOnlyChannels with shared StudioChannelCard and StudioChannelsPage layout
Thorough test suites using @testing-library/vue covering dropdown menus, modal flows, navigation, and conditional rendering
Good extraction of useChannelList composable for shared channel-loading logic

Note: As mentioned in the PR description, the temporary KDS fork in package.json needs to be reverted before merge.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Reviewed the pull request diff checking for:

Correctness: bugs, edge cases, undocumented behavior, resource leaks, hardcoded values
Design: unnecessary complexity, naming, readability, comment accuracy, redundant state
Architecture: duplicated concerns, minimal interfaces, composition over inheritance
Testing: behavior-based assertions, mocks only at hard boundaries, accurate coverage
Completeness: missing dependencies, unupdated usages, i18n, accessibility, security
Principles: DRY (same reason to change), SRP, Rule of Three (no premature abstraction)
Checked CI status and linked issue acceptance criteria
For UI changes: inspected screenshots for layout, visual completeness, and consistency

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioChannelCard/index.vue

contentcuration/contentcuration/frontend/channelList/composables/useChannelList.js

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioChannelsPage/index.vue

...ation/frontend/channelList/views/Channel/StudioMyChannels/__tests__/StudioMyChannels.spec.js

rtibblesbot

Clean follow-up addressing prior feedback. CI pending (linting, frontend build, frontend tests). Screenshots verified — all four pages look visually complete and consistent.

1 prior finding(s) resolved.

Suggestions:

Empty-state "No channels found" may not display when a user has zero channels (see inline comment on StudioChannelsPage)

Praise:

StudioStarredChannels tests nicely cover both editable and view-only channel behaviors within the same page, testing the conditional dropdown logic thoroughly

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioChannelsPage/index.vue

...tend/channelList/views/Channel/StudioStarredChannels/__tests__/StudioStarredChannels.spec.js

rtibblesbot

Clean delta — prior feedback addressed.

CI: linting and frontend tests passing; build frontend assets still pending. Screenshots previously verified.

2 prior finding(s) resolved.

Suggestions:

None

Reminders:

KDS fork in package.json still needs reverting before merge (as noted in PR description)

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Compared the current PR state against findings from a prior review:

Retrieved prior bot reviews deterministically via the GitHub API
Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
Only raised NEW findings for newly introduced code
Reviewed the pull request diff checking for correctness, design, architecture, testing, completeness, and adherence to DRY/SRP principles
Checked CI status and linked issue acceptance criteria

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioMyChannels/index.vue

marcellamaki · 2026-03-06T14:49:37Z

...tend/channelList/views/Channel/StudioStarredChannels/__tests__/StudioStarredChannels.spec.js

+      await userEvent.click(within(menu).getByText('Copy channel token'));
+      await waitFor(() => {
+        expect(
+          screen.getByText('Paste this token into Kolibri to import this channel'),


this isn't blocking and is just a question as we've been discussing tests across the ecosystem and VTL (and this makes me think about one other PR that I reviewed a few weeks ago). It seems like many of these tests introduced in this PR are getByText which seems to be a new pattern. it seems mostly to just exist unstable around new tests. Of course, adding tests has been a helpful part of the community contributor work. At the same time, think that for some of these new tests, it's a level of granularity that isn't necessarily useful.

at this point in the work, I don't think rewriting the tests is necessary. But I do think that this pattern will continue to be used as people see it for reference, especially community contributors. Would there be anything helpful we can do around decisions about VTL and data-testid that would be helpful to update in documentation? Also -- it's possible I missed something about this pattern, I did look through slack and only saw the discussion regarding data-testid and pointing people to the VTL docs.

@marcellamaki I think there was a confusion that I might have contributed to, apologies. data-testid is indeed useful, but that guidance is specific to contexts when it's not possible to use other queries, or when other approaches result in complicated test setup (e.g. many stubs).

Before we even get to data-testid there are many other options. I think it'd be best to just use this guidance? https://testing-library.com/docs/queries/about/#priority

And then there's another discussion we had around data-test vs data-testid ~ but again that would too apply only when we have no other option than data test ID. Happy to chat about this more.

Thanks, Misha. I can definitely understand that data-testid isn't suitable for all options, but this is helpful -- I do think I was a bit confused, but it's clearer now :)

marcellamaki

@MisRob thanks for the work on this and @bjester and @akolson for the thoughtful feedback. I went through and did manual QA and did some code review as well, and it looks like everything has been addressed. I did add one non-blocking comments about tests, but it's more of a note for a future conversation not something to resolve right now.

MisRob · 2026-03-06T15:38:31Z

Pushed one last commit to resolve #5696 (comment) & will merge briefly so we can start QA! Thanks all :)

yeshwanth235 and others added 14 commits November 7, 2025 13:51

[Remove Vuetify from Studio] Cards in My Channels

0be0e93

[Remove Vuetify from Studio] Cards in Starred channels changes

ad0d85f

[Remove Vuetify from Studio] Cards in View-only channels changes

b7c3b45

Merge branch 'unstable' into channel-cards

2213979

Merge pull request learningequality#5657 from MisRob/channel-cards

d3f3a17

Unstable to channel cards branch

Merge branch 'unstable' into channel-cards-finalize

fbfe7c6

Use 16:9 ratio for thumbnails

07c0e13

Remove unnecessary style definition

04f1ab8

KDS has recently started providing visuallyhidden so is not needed.

Fix token reference

d69002a

Do not add KTooltip to DOM when not necessary

ec17f60

Abstract page layout into a component

32093d9

and related re-organization of files and tests. Also adds more tests to some components.

Move invitations box to StudioMyChannels

217c912

[Remove Vuetify from Studio] Cards in Content Library

6840bdd

Merge branch 'channel-cards-finalize' into channel-cards

c277e85

bjester self-requested a review February 10, 2026 16:43

rtibbles assigned marcellamaki Feb 10, 2026

rtibbles requested review from akolson and marcellamaki February 10, 2026 16:43

Remove handler call

eefe23f

MisRob force-pushed the channel-cards branch from fb06598 to bec64b0 Compare February 10, 2026 18:54

MisRob mentioned this pull request Feb 10, 2026

KCard and KImg updates in support of Studio's channel cards learningequality/kolibri-design-system#1203

Merged

MisRob force-pushed the channel-cards branch 7 times, most recently from e5ef964 to 6588fd0 Compare February 13, 2026 15:02

MisRob added 2 commits February 14, 2026 10:58

Configure skeleton loaders to reflect

5327a26

shape of loaded cards.

Add bottom padding

36f359e

bjester reviewed Feb 17, 2026

View reviewed changes

akolson reviewed Feb 19, 2026

View reviewed changes

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioChannelsPage/index.vue Show resolved Hide resolved

akolson reviewed Feb 19, 2026

View reviewed changes

contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogList.vue Outdated Show resolved Hide resolved

akolson reviewed Feb 19, 2026

View reviewed changes

contentcuration/contentcuration/frontend/channelList/views/Channel/CatalogList.vue Outdated Show resolved Hide resolved

akolson reviewed Feb 19, 2026

View reviewed changes

MisRob added 3 commits February 27, 2026 19:07

Remove modals from card component

f8c7d53

to prevent unnecessary memory usage and to improve architecture. Relatedly remove props for configuring footer in favor of the footer slot to make communication between the modals and footer buttons straightforward.

Use StudioCopyToken instead of CopyToken

f780f83

in channel token modal and remove unnecessary token tracking handlers from components (StudioCopyToken handles tracking)

Fix invitation box width regression

52535eb

MisRob commented Feb 27, 2026

View reviewed changes

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioMyChannels/index.vue Outdated Show resolved Hide resolved

MisRob added 6 commits March 4, 2026 13:14

Use location.assign

57e8806

Cleanup styles

93d02ea

Use method instead of inline handler

92bf22b

Use location.assign

f278d6a

Cleanup uneccesarry key

f51d15a

Fix catalog channel details URL

a2ddd21

MisRob requested a review from rtibblesbot March 4, 2026 13:15

rtibblesbot reviewed Mar 4, 2026

View reviewed changes

Fix missing dir and notranslate

f59ebd5

MisRob requested a review from rtibblesbot March 4, 2026 13:33

rtibblesbot reviewed Mar 4, 2026

View reviewed changes

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioChannelsPage/index.vue Show resolved Hide resolved

...tend/channelList/views/Channel/StudioStarredChannels/__tests__/StudioStarredChannels.spec.js Show resolved Hide resolved

Render slot conditionally to ensure empty message displayes properly

82f6de2

MisRob requested a review from rtibblesbot March 4, 2026 13:48

rtibblesbot approved these changes Mar 4, 2026

View reviewed changes

contentcuration/contentcuration/frontend/channelList/views/Channel/StudioMyChannels/index.vue Show resolved Hide resolved

marcellamaki reviewed Mar 6, 2026

View reviewed changes

marcellamaki approved these changes Mar 6, 2026

View reviewed changes

Handle error state

eeef03b

MisRob merged commit 4665226 into learningequality:unstable Mar 6, 2026
13 checks passed

MisRob mentioned this pull request Mar 9, 2026

Don't show pagination when loading #5747

Merged

Conversation

MisRob commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

References

Reviewer guidance

Notes

Uh oh!

bjester left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MisRob commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

akolson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marcellamaki Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

MisRob Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

MisRob Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

marcellamaki Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

marcellamaki left a comment

Choose a reason for hiding this comment

Uh oh!

MisRob commented Mar 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

MisRob commented Feb 10, 2026 •

edited

Loading

MisRob commented Feb 18, 2026 •

edited

Loading