frontend: kraken: prevent container-not-up icon from overlapping update button#3883
frontend: kraken: prevent container-not-up icon from overlapping update button#3883nicoschmdt wants to merge 1 commit intobluerobotics:masterfrom
Conversation
Reviewer's GuideRefactors the "container not running" status indicator on the InstalledExtensionCard to be a Vuetify badge overlaid on the extension logo instead of an absolutely positioned icon, encapsulating the visibility logic in a computed property and updating styles accordingly. Class diagram for InstalledExtensionCard container_not_up refactorclassDiagram
class InstalledExtensionCard {
<<VueComponent>>
+extension
+extensionData
+container
+loading
+buttonBgColor() string
+container_not_up() boolean
+update_available() false|string
}
class ContainerNotUpBadge {
<<UIElement>>
+v-badge
+v-avatar
+v-img
+v-tooltip
+overlap
+offset_x
+offset_y
+color
+icon
+class container-not-up-badge
}
InstalledExtensionCard --> ContainerNotUpBadge : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider renaming the
container_not_upcomputed property tocontainerNotUp(or similar) to match typical JavaScript/Vue camelCase naming conventions for reactive properties. - Instead of passing an empty string to
v-tooltipwhencontainer_not_upis false, gate the tooltip withv-if/v-showor move the condition into the directive so the tooltip isn’t instantiated with empty content.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider renaming the `container_not_up` computed property to `containerNotUp` (or similar) to match typical JavaScript/Vue camelCase naming conventions for reactive properties.
- Instead of passing an empty string to `v-tooltip` when `container_not_up` is false, gate the tooltip with `v-if`/`v-show` or move the condition into the directive so the tooltip isn’t instantiated with empty content.
## Individual Comments
### Comment 1
<location path="core/frontend/src/components/kraken/cards/InstalledExtensionCard.vue" line_range="15" />
<code_context>
- size="60"
- class="mr-3"
- rounded="0"
+ v-tooltip="container_not_up ? 'This extension is enabled but the container is not running.' : ''"
+ :value="container_not_up"
+ overlap
</code_context>
<issue_to_address>
**suggestion:** Avoid passing an empty string to v-tooltip when the container is up.
Rather than returning an empty string in the ternary, conditionally apply the tooltip only when `container_not_up` is true, e.g. `v-tooltip="container_not_up && 'This extension is enabled but the container is not running.'"` or by wrapping the tooltip in `v-if="container_not_up"`. This prevents creating a tooltip with an empty label, which can lead to odd UX or accessibility behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| size="60" | ||
| class="mr-3" | ||
| rounded="0" | ||
| v-tooltip="container_not_up ? 'This extension is enabled but the container is not running.' : ''" |
There was a problem hiding this comment.
suggestion: Avoid passing an empty string to v-tooltip when the container is up.
Rather than returning an empty string in the ternary, conditionally apply the tooltip only when container_not_up is true, e.g. v-tooltip="container_not_up && 'This extension is enabled but the container is not running.'" or by wrapping the tooltip in v-if="container_not_up". This prevents creating a tooltip with an empty label, which can lead to odd UX or accessibility behavior.

looks like this now:

Summary by Sourcery
Indicate the container-not-running state on the installed extension card using a badge over the extension logo instead of an overlapping standalone icon.
Enhancements: