-
Notifications
You must be signed in to change notification settings - Fork 191
feat: enhance inspect panel, add copy visual info for agents #928
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
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates across several packages: a generated CSS export file had a non-functional identical-line edit; NuxtDevtoolsInspectPanel.vue introduces toast UI state, utilities ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
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.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue`:
- Around line 282-285: The toast's exit animation is never applied because the
div uses v-if="showToast" which removes the element from the DOM; change the
template to either use v-show="showToast" on the same div so the :class branch
('translate-y-10 op0') can be applied on hide, or wrap the div in a <Transition>
component and move the entering/leaving classes there so showToast still
controls visibility but the element remains in DOM long enough to animate;
update the template where the div with class binding and v-if is defined (the
element referencing showToast and :class) to use v-show or a Transition wrapper
accordingly.
- Around line 104-113: The selector-building logic computes an index among
same-tag siblings (variables: current, parent, siblings) but then appends
:nth-child(index), which counts all children and can select the wrong element;
change the appended pseudo-class from `:nth-child(${index})` to
`:nth-of-type(${index})` so the generated selector matches the element
type-filtering already performed by the siblings array and keeps the rest of the
logic (parent, siblings, selector) unchanged.
- Around line 235-243: The v-if condition on the Parent button is inverted
causing it to render only when no parent exists; update the condition in
NuxtDevtoolsInspectPanel.vue so the button renders when props.hasParent is true
(replace the current v-if="!props.hasParent" with v-if="props.hasParent") so the
"Go to parent" button (which calls selectParent) only appears when a parent is
available.
- Around line 200-213: The toast timeout isn't cleared when copyAgentInfo is
called repeatedly, causing earlier timers to hide the toast prematurely; add a
module-level variable (e.g., toastTimeoutId) to hold the timeout ID, call
clearTimeout(toastTimeoutId) before scheduling the new setTimeout, assign the
new timeout ID to toastTimeoutId when you call setTimeout, and optionally reset
toastTimeoutId to undefined when the timeout fires or when showToast is set
false; update references in copyAgentInfo and any toast-hide logic (showToast,
toastContent) accordingly.
🧹 Nitpick comments (2)
packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue (2)
126-165: Looks good overall — defensivetry/catcharoundgetParent()is a nice touch.Two minor observations:
buildComponentTree()is called directly in the template (Line 276), so it re-executes on every render. Consider memoizing it with acomputedor caching the result whenprops.matchedchanges, though in practice the impact here is negligible since the panel is short-lived.- The
as anycasts on internal Vue properties (__name,__file) are inherently fragile across Vue versions — worth a brief inline comment noting this.
219-226: Conflictingfixedandrelativeposition classes on the same element.Line 221 applies both
fixedandrelativeutility classes. While the inlinestylefromuseDraggableoverrides thepositionproperty, having both classes is confusing. Consider removing the redundant one.
packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue
Show resolved
Hide resolved
packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue
Show resolved
Hide resolved
packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue
Show resolved
Hide resolved
packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue`:
- Around line 98-103: The selector-building logic that appends classes from
current.className currently joins raw class names (in the block handling
current.className) which produces invalid selectors for classes with special
characters; update that code to split and filter classes as before but escape
each class name with CSS.escape() (or alternatively filter out classes
containing non-alphanumeric characters) before joining with '.' so generated
selectors are valid (target the block using current.className and the variable
selector to apply the fix).
🧹 Nitpick comments (2)
packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue (2)
277-279:buildComponentTree()called directly in the template will re-execute on every render.This function walks the entire parent chain on every re-render. Extract it to a
computedso it only recalculates whenprops.matchedchanges:Suggested refactor
In
<script setup>:+const componentTree = computed(() => buildComponentTree())In
<template>:- <span class="break-all text-xs font-mono" title="Component Tree">{{ buildComponentTree() }}</span> + <span class="break-all text-xs font-mono" title="Component Tree">{{ componentTree }}</span>
222-229: Dead ternary branch —v-ifalready removes the element when!props.matched.Line 222 uses
v-if="props.matched", so line 228's falsy branch ('op0 pointer-events-none') is unreachable. You can simplify this to just the truthy class or remove the ternary entirely.
packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue
Show resolved
Hide resolved
…pectPanel.vue Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue`:
- Around line 92-96: The code builds a CSS selector using current.id without
escaping; update the selector construction to use CSS.escape(current.id) so IDs
with special chars (e.g., '.', ':') produce valid selectors — replace the
interpolation `#${current.id}` with `#${CSS.escape(current.id)}` in the block
that sets selector and calls path.unshift(selector) (ensure CSS.escape is
available or polyfilled where this runs).
🧹 Nitpick comments (3)
packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue (3)
272-280:buildComponentTree()is invoked directly in the template — consider caching it in a computed or ref.Every re-render re-walks the entire parent chain. While acceptable for a devtools panel, a
computed(or updating a ref whenprops.matchedchanges) would avoid redundant traversals and keep the template declarative.
213-216: Silent failure — user sees nothing when clipboard write is denied.If
navigator.clipboard.writeTextrejects (e.g., permissions policy, non-secure context), the catch only logs to the console. Consider surfacing the error in the toast or via a brief visual indicator so the user knows the copy didn't succeed.Sketch
catch (error) { console.error('Failed to copy agent info:', error) + toastContent.value = 'Failed to copy to clipboard' + showToast.value = true + if (toastTimer) + clearTimeout(toastTimer) + toastTimer = setTimeout(() => { + showToast.value = false + }, 4_000) }
222-229: Dead branch in:class— thev-ifalready removes the element whenmatchedis falsy.Since
v-if="props.matched"on Line 222 guarantees the element only exists whenmatchedis truthy, the'op0 pointer-events-none'branch on Line 228 is unreachable. You can simplify the binding.:class="[ isDragging ? 'transition-none' : 'transition-opacity', - props.matched ? 'op100' : 'op0 pointer-events-none', ]"
packages/devtools/src/webcomponents/components/NuxtDevtoolsInspectPanel.vue
Show resolved
Hide resolved
…pectPanel.vue Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
atinux
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.
Let's go 🚀
Follow https://vercel.com/changelog/copy-visual-context-to-agents, this PR enhanced the inspect panel to show more info and an additional button
Copy Info for agents