learningequality
[Remove Vuetify from Studio] Channel details in Channels - content
Summary
Migrates ChannelDetailsModal from Vuetify to Kolibri Design System (KDS).
Components
- StudioDetailsRow.vue - CSS Grid layout replacing VLayout/VFlex
- StudioDetailsPanel.vue - Complete KDS migration with KImg, KTooltip, semantic HTML
- Uses existing StudioChip from unstable (contributor's implementation)
Key Changes
- Replaced Vuetify components (VCard, VLayout, VChip, VTooltip) with KDS equivalents
- KImg with 16:9 aspect ratio for thumbnails + placeholder
Testing
- 41 tests passing (StudioDetailsPanel: 28, StudioDetailsRow: 13)
- Data-present vs data-absent scenarios for all fields
- Following VTL best practices - user-facing behavior only
Screen-Recording:
https://github.com/user-attachments/assets/73d1a49c-235e-405a-bf3a-49b4bd78482c
…
References
fixes : #5530 …
Reviewer guidance
- Check RTL language support
- Run unit tests: npm test StudioChip StudioDetailsRow StudioDetailsPanel
…
Hi @vtushar06, thank you. As for the structure of components, and very high-level, looks as expected :+1:
Before we dive into full review though, let's revisit approach to testing. This will help a reviewer too - they will be able to recognize whether the logic behaves as expected for channels with/without all sorts of data.
Also, the guidance I left is quite general. I know you have more PRs open - would you please check if there are some similar patterns to be addressed there as well?
Hii @MisRob, I’ve updated the tests to Tests and refined them so they now only check what actually matters. Please let me know if this looks good to you or if anything else needs to be adjusted.
One more thing when I tested this in 'rtl' I can see that notranslate class doesn't follow that on official website and same on localhost, what should I do to that??
Hi @vtushar06,
We will assign reviewer here week or two later, some time after we've merged few other PRs, one of them yours, that will be good to have in place before we check these changes.
Meanwhile you can revisit:
(1) The use of stubs and possible simplification of the test suite setup according to notes I left today on your other PRs, if relevant.
(2) As for your note:
Data-present vs data-absent scenarios for all fields.
I don't yet see this in StudioDetailsPanel.spec.js - would definitely be helpful since this component has lots of conditional logic. This will take some effort - take your time.
One more thing when I tested this in 'rtl' I can see that notranslate class doesn't follow that on official website and same on localhost, what should I do to that??
I don't quite understand the question - would you post a screenshot, or rephrase?
Hi @vtushar06, I wanted to mention that I've just merged another PR that introduces StudioChip and I think it'd be best to re-use the existing implementation rather than having two components. Sorry for this timing, I didn't realize it was already work in progress elsewhere when I assigned you this issue.
I think it'd be easiest if you rebased this PR on top of the latest unstable branch, and then fully accept the incoming unstable's StudioChip, and use it for your work here, rather than adding new one. No worries about slightly different round corners - just use StudioChip as is and then we will see if some adjustments are needed. This will also resolve the conflict.
Hi @MisRob, I've completed all the requested changes - tests now follow VTL principles with data-present/absent scenarios, and I'm using the contributor's StudioChip from unstable. About RTL/notranslate: The notranslate class is for excluding user content from translation services, not for text direction. The dir="auto" attribute handles RTL direction. Is this the expected behavior? and all 41 tests passing. Now it is ready for review .
Hii @MisRob, Please give me some time I am going through all the files line by line to see any cleanups require or any other change referencing to my other PRs.
Much appreciated @vtushar06 - and take all the time you need, no rush. We need to finish other reviews at first anyway.
@vtushar06, before you revisit it, would you also merge the latest unstable here and resolved conflicts? Hopefully it will be straightforward - I think the conflicting lines come from the other PR from you that we recently merged.
yeah sure @MisRob, I will and work is almost done, last commit will be pushed very soon.
Hi @MisRob, I have merged the latest unstable and resolved all conflicts.Now PR is ready for review.
Hi @vtushar06, I'm sorry for delay - I will review after I'm back from my vacation. It'll be quite a long time - just in case you wouldn't be around later in January or February, I can just push requested changes, if I locate any, too. Thanks!
sure @MisRob, I will be there waiting for your review and you can push changes if exists.
Thanks for your patience @vtushar06. Take care and see you in the new year :)!