Skip to content

Conversation

@vtushar06
Copy link
Contributor

@vtushar06 vtushar06 commented Oct 28, 2025

Summary

Migrates ChannelDetailsModal from Vuetify to Kolibri Design System by creating a new StudioImmersiveModal component.

Changes:

  • Created StudioImmersiveModal.vue - Fullscreen modal using KToolbar and KIconButton (no Vuetify dependencies)
  • Refactored ChannelDetailsModal.vue:
    • Replaced FullscreenModal with StudioImmersiveModal
    • Replaced Vuetify download menu with KButton + KDropdownMenu
    • Replaced LoadingText with StudioLargeLoader
    • Removed all Vuetify components (VCard, VLayout, VSpacer, VBtn, VList, etc.)
    • Preserved DetailsPanel unchanged per requirements

Screenshots:

Screenshot 2025-11-03 at 2 50 23 PM Screenshot 2025-11-03 at 2 50 10 PM Screenshot 2025-10-27 at 9 21 36 AM

References

Closes: #5474

Reviewer guidance

Quick Test:

  1. pnpm install && pnpm run devserver:hot
  2. Login as [email protected] / a
  3. Go to Channels > My Channels
  4. Click info icon (ⓘ) on any channel
  5. Verify: toolbar, close button, ESC key, download dropdown, responsive layout

Key Test Cases:

  • Close button & ESC key work
  • Download PDF/CSV functional
  • Mobile: full-width button
  • RTL: proper spacing and alignment (pnpm run devserver)
  • Offline alert appears when disconnected

@vtushar06 vtushar06 marked this pull request as ready for review October 30, 2025 05:45
@vtushar06
Copy link
Contributor Author

Hii @MisRob,question about layout:

Using StudioPage as specified, but its margin-top: 104px creates extra spacing in the fullscreen modal (we only have a 64px toolbar).
should I use:

  1. Use custom layout instead(currently used)
  2. Keep as-is

Let me know your preference! Thanks!

@MisRob
Copy link
Member

MisRob commented Oct 31, 2025

Hi @vtushar06,

I think it'd be meaningful to remove margin-top: 104px; from StudioPage internal implementation, and let it be configured from outside via class or style, for example:

<StudioPage
  :style={ marginTop: ...}
/>

In this way, we will be able to use StudioPage in all current and future contexts easily.

Note that there are few other places where StudioPage is used. After you have done all changes, would you please check that they still work as expected both with and without the offline alert bar?

… structure; enhance ChannelDetailsModal tests for better clarity and functionality
@vtushar06 vtushar06 force-pushed the fix/channel-details-remove-vuetify-5474 branch from 16caaa3 to 3ced5cf Compare November 3, 2025 03:12
@vtushar06 vtushar06 force-pushed the fix/channel-details-remove-vuetify-5474 branch from 1453e81 to c2a3e42 Compare November 3, 2025 04:27
@vtushar06
Copy link
Contributor Author

Hi @MisRob,
I’ve implemented your feedback regarding the StudioPage marginTop prop and I believe the PR is now ready for review.

@vtushar06
Copy link
Contributor Author

I wanted to confirm the intended behavior: I’ve added margin: '0 auto' to StudioPage’s inner content to center the max-width: 1000px container on large screens. This improves the layout for the modal and other pages using StudioPage.
This change affects all pages using StudioPage, but should be a visual improvement (centered content vs left-aligned). I’ve tested locally and it looks good. Should I verify anything specific regarding other pages using StudioPage?

@MisRob
Copy link
Member

MisRob commented Nov 3, 2025

@vtushar06 we won't do any changes to user experience unless explicitly mentioned in the issue - it's not the goal of this project, and we have internal processes that would need to precede such decisions. So please stay as close as possible to the current experience.

@vtushar06
Copy link
Contributor Author

okay sure @MisRob, I thought in sake for UX, reverting back the changes to use current experience, thanks for this guidance too.

@vtushar06 vtushar06 force-pushed the fix/channel-details-remove-vuetify-5474 branch from 3ccd761 to 5437d8a Compare November 3, 2025 09:21
@vtushar06
Copy link
Contributor Author

Hii @MisRob, I have updated inner page to use previous codes, I think now you can review this out whenever possible and let me know if any further changes required to update.

@MisRob
Copy link
Member

MisRob commented Nov 3, 2025

Thanks for you understanding @vtushar06 - it's definitely good that you think about how to improve user experience - I just need to work within the scope of this project ;)

@MisRob MisRob self-requested a review November 4, 2025 09:28
@MisRob MisRob self-assigned this Nov 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants