fix(sidepanel): fill artifact viewer height#1442
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request refactors the component layout hierarchy to fix an issue where the artifact code pane doesn't take the full available window height. The changes apply flexbox constraints ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/renderer/src/components/sidepanel/viewer/WorkspaceCodePane.vue (1)
283-299: Prefer not to hide Monaco sizing rules in a scoped!importantblock.This fix now depends on component-local deep selectors and
!importantoverrides, which makes the layout harder to reason about and maintain than the Tailwind-based template changes above. If these overrides are unavoidable, I'd move them into a shared Monaco/editor stylesheet instead of keeping them embedded in one pane component. As per coding guidelinessrc/renderer/src/**/*.vue: Use Tailwind for styling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/sidepanel/viewer/WorkspaceCodePane.vue` around lines 283 - 299, The component WorkspaceCodePane.vue currently embeds scoped CSS with deep selectors and multiple !important rules for .workspace-code-editor-host and Monaco subselectors; remove these scoped/deep/!important style overrides and instead move equivalent layout rules into a shared, non-scoped Monaco/editor stylesheet (e.g., a global CSS or shared tailwind-compatible file) so Monaco sizing is controlled globally; update the template to rely on Tailwind utility classes on the .workspace-code-editor-host element and ensure the shared stylesheet targets the Monaco classes (.monaco-editor, .overflow-guard, .monaco-scrollable-element) without using :deep or !important so the layout is consistent across panes.src/renderer/src/components/artifacts/SvgArtifact.vue (1)
9-9: Optional: deduplicate repeated state container classes.Loading, error, and empty states reuse the same class string; extracting it to a constant/class binding would reduce drift risk in future tweaks.
Also applies to: 18-18, 35-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/components/artifacts/SvgArtifact.vue` at line 9, The repeated state container class string used in the template of SvgArtifact.vue for loading, error, and empty states should be extracted to a single source to avoid drift; add a constant or computed property (e.g., containerClass or stateContainerClass) inside the SvgArtifact component script and replace each hard-coded class="flex min-h-full w-full flex-1 flex-col items-center justify-center p-8 text-center" with a binding :class="containerClass" (or use a named CSS utility class and reference it) so all three places (the loading, error, and empty state blocks) share the same value.test/renderer/components/WorkspacePreviewPane.test.ts (1)
85-87: Consider deduplicating repeated pane layout assertions.The same
workspace-preview-paneclass expectation appears in multiple tests; extracting a helper/constant will make updates less brittle.Example refactor
+const expectedPreviewPaneClasses = [ + 'flex', + 'h-full', + 'min-h-0', + 'w-full', + 'flex-col', + 'overflow-hidden' +] + +const expectPreviewPaneLayout = (wrapper: ReturnType<typeof mount>) => { + expect(wrapper.get('[data-testid="workspace-preview-pane"]').classes()).toEqual( + expect.arrayContaining(expectedPreviewPaneClasses) + ) +} ... - expect(wrapper.get('[data-testid="workspace-preview-pane"]').classes()).toEqual( - expect.arrayContaining(['flex', 'h-full', 'min-h-0', 'w-full', 'flex-col', 'overflow-hidden']) - ) + expectPreviewPaneLayout(wrapper)Also applies to: 150-152, 199-201
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/renderer/components/WorkspacePreviewPane.test.ts` around lines 85 - 87, Extract the repeated assertion of classes on wrapper.get('[data-testid="workspace-preview-pane"]').classes() into a shared helper or constant used by tests to avoid duplication; for example, add a constant like EXPECTED_WORKSPACE_PREVIEW_PANE_CLASSES (or a helper function assertWorkspacePreviewPaneLayout(wrapper)) and replace the repeated expect(...).toEqual(expect.arrayContaining([...])) calls in WorkspacePreviewPane.test.ts (occurring around the existing checks and at the other noted locations) with calls to that constant/helper so updates only need to change one place.test/renderer/components/ReactArtifact.test.ts (1)
7-16: Avoid mounting todocument.bodywhen it isn’t needed.This test only checks classes, so
attachTo: document.bodyadds avoidable DOM coupling and potential residue between tests.Proposed simplification
const wrapper = mount(ReactArtifact, { props: { block: { content: 'export default function App() { return <div>Hello</div> }', artifact: { type: 'application/vnd.ant.react', title: 'App' } }, isPreview: true - }, - attachTo: document.body + } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/renderer/components/ReactArtifact.test.ts` around lines 7 - 16, The test for ReactArtifact mounts the component with attachTo: document.body which creates unnecessary DOM coupling; update the test to remove the attachTo option (or use shallowMount if you only need class/CSS assertions) so the wrapper is mounted in the default JSDOM container and won't leave residues between tests—modify the mount call in the ReactArtifact.test (the mount(...) invocation) to omit attachTo: document.body or replace mount with shallowMount when appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/src/components/artifacts/MermaidArtifact.vue`:
- Around line 77-79: The current global regex that mutates the sanitized string
(the line assigning sanitized =
sanitized.replace(/on\w+\s*=\s*(?:"[^"]*"|'[^']*'|[^\s>]+)/gi, '')) removes on*
attributes from the entire Mermaid source and can strip valid diagram text like
"oncall=alice"; restrict the scrubber to only operate inside HTML tags by first
matching tag contents (e.g. use sanitized.replace(/<[^>]+>/g, tag => /* remove
on* attributes inside tag */)) and then removing on* attributes only within
those matched tag strings (i.e., run the on\w+... remover against the tag
substring in the replace callback), leaving plain Mermaid text untouched.
---
Nitpick comments:
In `@src/renderer/src/components/artifacts/SvgArtifact.vue`:
- Line 9: The repeated state container class string used in the template of
SvgArtifact.vue for loading, error, and empty states should be extracted to a
single source to avoid drift; add a constant or computed property (e.g.,
containerClass or stateContainerClass) inside the SvgArtifact component script
and replace each hard-coded class="flex min-h-full w-full flex-1 flex-col
items-center justify-center p-8 text-center" with a binding
:class="containerClass" (or use a named CSS utility class and reference it) so
all three places (the loading, error, and empty state blocks) share the same
value.
In `@src/renderer/src/components/sidepanel/viewer/WorkspaceCodePane.vue`:
- Around line 283-299: The component WorkspaceCodePane.vue currently embeds
scoped CSS with deep selectors and multiple !important rules for
.workspace-code-editor-host and Monaco subselectors; remove these
scoped/deep/!important style overrides and instead move equivalent layout rules
into a shared, non-scoped Monaco/editor stylesheet (e.g., a global CSS or shared
tailwind-compatible file) so Monaco sizing is controlled globally; update the
template to rely on Tailwind utility classes on the .workspace-code-editor-host
element and ensure the shared stylesheet targets the Monaco classes
(.monaco-editor, .overflow-guard, .monaco-scrollable-element) without using
:deep or !important so the layout is consistent across panes.
In `@test/renderer/components/ReactArtifact.test.ts`:
- Around line 7-16: The test for ReactArtifact mounts the component with
attachTo: document.body which creates unnecessary DOM coupling; update the test
to remove the attachTo option (or use shallowMount if you only need class/CSS
assertions) so the wrapper is mounted in the default JSDOM container and won't
leave residues between tests—modify the mount call in the ReactArtifact.test
(the mount(...) invocation) to omit attachTo: document.body or replace mount
with shallowMount when appropriate.
In `@test/renderer/components/WorkspacePreviewPane.test.ts`:
- Around line 85-87: Extract the repeated assertion of classes on
wrapper.get('[data-testid="workspace-preview-pane"]').classes() into a shared
helper or constant used by tests to avoid duplication; for example, add a
constant like EXPECTED_WORKSPACE_PREVIEW_PANE_CLASSES (or a helper function
assertWorkspacePreviewPaneLayout(wrapper)) and replace the repeated
expect(...).toEqual(expect.arrayContaining([...])) calls in
WorkspacePreviewPane.test.ts (occurring around the existing checks and at the
other noted locations) with calls to that constant/helper so updates only need
to change one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9b812bf-bb8e-4a44-ac05-f0f800a90059
📒 Files selected for processing (18)
src/renderer/src/App.vuesrc/renderer/src/components/artifacts/HTMLArtifact.vuesrc/renderer/src/components/artifacts/MermaidArtifact.vuesrc/renderer/src/components/artifacts/ReactArtifact.vuesrc/renderer/src/components/artifacts/SvgArtifact.vuesrc/renderer/src/components/sidepanel/ChatSidePanel.vuesrc/renderer/src/components/sidepanel/WorkspacePanel.vuesrc/renderer/src/components/sidepanel/WorkspaceViewer.vuesrc/renderer/src/components/sidepanel/viewer/WorkspaceCodePane.vuesrc/renderer/src/components/sidepanel/viewer/WorkspaceInfoPane.vuesrc/renderer/src/components/sidepanel/viewer/WorkspacePreviewPane.vuesrc/renderer/src/views/ChatTabView.vuetest/renderer/components/HTMLArtifact.test.tstest/renderer/components/MermaidArtifact.test.tstest/renderer/components/ReactArtifact.test.tstest/renderer/components/SvgArtifact.test.tstest/renderer/components/WorkspacePreviewPane.test.tstest/renderer/components/WorkspaceViewer.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix #1438
Summary by CodeRabbit
Bug Fixes
Tests