Implement majority of Resource Detail Cards#16364
Implement majority of Resource Detail Cards#16364codyrancher merged 14 commits intorancher:masterfrom
Conversation
| }; | ||
| } | ||
|
|
||
| get resourceConditions() { |
There was a problem hiding this comment.
This was moved from https://github.com/rancher/dashboard/pull/16364/files#diff-4bca3d578567d2f2f043426415b17c810870ceaed67a2b84aff3c27393c65012
I move it here since we want to the data both in cards and in the conditions table.
cbc0b64 to
30ce076
Compare
There was a problem hiding this comment.
The changes here were mostly moved from workload/index.vue since the pods/jobs cards are defined here.
af14878 to
a63345d
Compare
7c3c99f to
6a2c64f
Compare
…ms with our `Validate Plugin build system` step.
| <script setup lang="ts"> | ||
| const { resource } = defineProps<Props>(); | ||
| const cards = computed(() => resource?.cards?.filter((c: any) => c) || []); | ||
| const showExtrasCard = computed(() => cards.value.length >= 1 && cards.value.length < 3); |
There was a problem hiding this comment.
It's really a filler card to show if there are some cards but not many.
There was a problem hiding this comment.
You'll notice several examples of extracting the types from a .vue file. Even though the double script block seems to fully work in both dev and prod it seems to fail in our Validate Plugin build system CI and I can't figure out why so I've just removed this usage since it's not very common anyway.
There was a problem hiding this comment.
Replaced with the RcCounterBadge.
There was a problem hiding this comment.
There's several examples of imports/exports changing in rancher-components. This is because some of these components have never been included in shell and when doing so Validate Plugin build system would fail. There appears to be a subtle alias bug that I haven't been able to pin down so I've changed some of the imports/exports to resolve this problem.
| @@ -19,7 +20,7 @@ export interface Props { | |||
| const store = useStore(); | |||
There was a problem hiding this comment.
We may want to get rid of the double script block but it's not one that ever failed CI. I believe it's because it's only ever imported dynamically so typescript doesn't catch this at compile time.
| class="mmt-4" | ||
| /> | ||
| <Cards | ||
| v-if="props.isCustomDetailOrEdit" |
There was a problem hiding this comment.
We only ever want to render the cards when there's a custom detail page. If we render for all mastheads there's scenarios where we render yaml and we still see this even though the cards are supposed to reflect the tables/tabs that are on the page.
| }, | ||
|
|
||
| methods: { | ||
| async scale(isUp) { |
There was a problem hiding this comment.
These got moved into the model since they'll be used when we instantiate the cards in the model.
There was a problem hiding this comment.
Pull request overview
Implements the new card-based layout for workload resource detail pages, replacing the previous gauge-driven UI and introducing a reusable “cards” infrastructure across resources.
Changes:
- Added a generic card system (
cards/_cards) on baseResource, including an Insights card backed by conditions/events. - Implemented workload-specific cards (pods/jobs) and moved workload “related services” logic into the workload model.
- Updated mastheads/detail pages/tests to render cards and updated UI pieces (e.g., StatusRow counter badge).
Reviewed changes
Copilot reviewed 29 out of 31 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| shell/plugins/dashboard-store/resource-class.js | Adds resourceConditions, resourceEvents, and base cards/Insights card support. |
| shell/plugins/dashboard-store/tests/resource-class.test.ts | Adds unit coverage for new Resource getters/cards. |
| shell/models/workload.js | Adds workload cards (pods/jobs), scaling helper method, and relatedServices model logic. |
| shell/models/tests/workload.test.ts | Adds unit coverage for scaling, related services, and cards getters. |
| shell/detail/workload/index.vue | Removes gauges/scaling UI and switches service matching to value.relatedServices; adds (currently unused) insights props wiring. |
| shell/detail/tests/workload.test.ts | Updates ingress matching tests to use value.relatedServices. |
| shell/components/form/Conditions.vue | Reuses the new resourceConditions mapping from the base Resource. |
| shell/components/ResourceDetail/Masthead/latest.vue | Conditionally renders Cards for custom detail/edit pages. |
| shell/components/ResourceDetail/Masthead/index.vue | Passes isCustomDetailOrEdit to the latest masthead. |
| shell/components/ResourceDetail/Masthead/tests/latest.test.ts | Adds tests for conditional Cards rendering. |
| shell/components/ResourceDetail/Masthead/tests/index.test.ts | Adds tests verifying isCustomDetailOrEdit prop wiring. |
| shell/components/Resource/Detail/StatusRow.vue | Replaces Bubble counter UI with RcCounterBadge. |
| shell/components/Resource/Detail/ResourceRow.vue | Refactors types out + layout tweaks for count display. |
| shell/components/Resource/Detail/ResourceRow.types.ts | New extracted TS types for ResourceRow. |
| shell/components/Resource/Detail/Masthead/index.vue | Adds Cards rendering to the custom-detail masthead. |
| shell/components/Resource/Detail/Masthead/tests/index.test.ts | Adds tests ensuring masthead renders/passes resource to Cards. |
| shell/components/Resource/Detail/Cards.vue | New component to render resource.cards plus optional Extras card. |
| shell/components/Resource/Detail/Card/tests/PodsCard.test.ts | Updates tests to target the renamed/reworked StatusCard. |
| shell/components/Resource/Detail/Card/StatusCard/index.vue | Generalizes PodsCard → StatusCard (title + resources) and keeps scaling UI. |
| shell/components/Resource/Detail/Card/StatusCard/composable.ts | Adds useScaling helper (currently unused). |
| shell/components/Resource/Detail/Card/StateCard/types.ts | New extracted TS types for StateCard props. |
| shell/components/Resource/Detail/Card/StateCard/index.vue | Refactors StateCard to use extracted props types. |
| shell/components/Resource/Detail/Card/StateCard/composables.ts | Enhances aggregation/sorting and adds useResourceCardRow flexibility. |
| shell/components/Resource/Detail/Card/StateCard/tests/composables.test.ts | Adds unit tests for useResourceCardRow. |
| shell/components/Resource/Detail/Card/ExtrasCard.vue | Adds “Extras” card with links to extensions/tools. |
| shell/components/Resource/Detail/Bubble.vue | Adds Bubble component (currently unused). |
| shell/assets/translations/en-us.yaml | Adds i18n strings for jobs/extras/insights cards. |
| pkg/rancher-components/src/index.ts | Exports new Pill components (counter/status/tag). |
| pkg/rancher-components/src/components/Pill/index.ts | Barrel export for Pill components. |
| pkg/rancher-components/src/components/Pill/RcCounterBadge/RcCounterBadge.vue | Fixes RcCounterBadge types import. |
| cypress/e2e/po/edit/resource-detail.po.ts | Updates Cypress PO selector from gauges to status rows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| get insightCardProps() { | ||
| const rows = [ | ||
| useResourceCardRow(this.t('component.resource.detail.card.insightsCard.rows.conditions'), this.resourceConditions, undefined, 'condition', '#conditions'), | ||
| useResourceCardRow(this.t('component.resource.detail.card.insightsCard.rows.events'), this.resourceEvents, undefined, undefined, '#events'), | ||
| ]; |
There was a problem hiding this comment.
useResourceCardRow(..., this.resourceEvents, ...) assumes event objects have stateDisplay and stateSimpleColor, but shell/models/event.js only defines eventType and does not provide those fields. This will aggregate under an undefined label/color and likely render incorrectly. Map events to a shape with the expected keys (or call useResourceCardRow with the correct keys, e.g. stateDisplayKey='eventType', plus a corresponding color field).
There was a problem hiding this comment.
@codyrancher I've initiated a code review by Copilot, could you please check the feedback?
There was a problem hiding this comment.
The Event model extends SteveModel which has stateDisplay and stateSimpleColor via inheritance.
| .count:not(.count + .count) { | ||
| max-width: calc(100% - 90px); | ||
|
|
||
| .count-label { | ||
| overflow: hidden; |
There was a problem hiding this comment.
.count:not(.count + .count) is a confusing selector for “first count item” and relies on a complex selector inside :not(). Prefer a clearer selector like .count:first-child (or apply the max-width rule via :first-of-type) to improve readability and compatibility.
There was a problem hiding this comment.
Unfortunately a simple selector isn't possible here. I started with first-child and first-of-type. Ultimately it's rather challenging to get the ellipsis and row the appropriate element to stretch without overflowing in this context.
shell/models/workload.js
Outdated
| title: this.t('component.resource.detail.card.podsCard.title'), | ||
| resources: this.pods, | ||
| 'show-scaling': this.canUpdate && scalingTypes.includes(this.type), | ||
| onIncrease: () => this.scale(true), | ||
| onDecrease: () => this.scale(false) |
There was a problem hiding this comment.
In this card.props object, the key is 'show-scaling', but StatusCard declares the prop as showScaling. When these props are passed via v-bind="card.props" in Cards.vue, using kebab-case keys in an object may not reliably map to camelCase props. Prefer using showScaling (camelCase) in the props object to ensure scaling toggles work.
| get resourceEvents() { | ||
| return this.$rootGetters['cluster/all'](EVENT); | ||
| } |
There was a problem hiding this comment.
resourceEvents returns all cluster Events and insightCardProps aggregates them. This will make the Insights card show counts unrelated to the current resource (and can be very expensive on large clusters). Consider filtering the events to this resource the same way the Events tab does (e.g., involvedObject.uid === this.metadata.uid, and special-case Namespace to match metadata.namespace).
| class="count" | ||
| > | ||
| {{ count.count }} {{ count.label }}<span class="and"> + </span> | ||
| <span class="count-value">{{ count.count }}</span> <div class="count-label">{{ count.label }}</div><span class="and"> + </span> |
There was a problem hiding this comment.
The template renders a <div class="count-label"> inside a <span class="count">, which is invalid HTML (block element inside inline element) and can cause inconsistent layout/accessibility behavior. Use inline elements (e.g. <span>) or change the outer wrapper to a block-level element.
| <span class="count-value">{{ count.count }}</span> <div class="count-label">{{ count.label }}</div><span class="and"> + </span> | |
| <span class="count-value">{{ count.count }}</span> <span class="count-label">{{ count.label }}</span><span class="and"> + </span> |
shell/detail/workload/index.vue
Outdated
| setup() { | ||
| const insightCardProps = useDefaultWorkloadInsightsCardProps(); | ||
|
|
||
| if (service.metadata?.namespace === this.value.metadata?.namespace && matches(pod, selector)) { | ||
| return true; | ||
| } | ||
| } | ||
| return { insightCardProps }; | ||
| }, |
There was a problem hiding this comment.
useDefaultWorkloadInsightsCardProps and the setup() return value (insightCardProps) are not referenced anywhere in this component. This is dead code now and should be removed to avoid confusion.
shell/detail/workload/index.vue
Outdated
| @@ -99,7 +93,6 @@ export default { | |||
| return { | |||
| servicesInNamespace: [], | |||
There was a problem hiding this comment.
servicesInNamespace remains in component data() but matchingServices/findMatchingServices were removed, so this state is now unused. Consider removing servicesInNamespace (and the corresponding cluster/findAll assignment in fetch()) to avoid extra work and confusion.
| servicesInNamespace: [], |
There was a problem hiding this comment.
fixed locally and pushed.
momesgin
left a comment
There was a problem hiding this comment.
I've tested the items mentioned in the PR's description, and the code LGTM too 🚀
Summary
Fixes #14581
Implements detail page cards for workload resources, replacing the gauge-based display with a card-based layout.
Note: it might be best to start at where are used:
Occurred changes and/or fixed issues
PodsCardtoStatusCardto support both pods and jobsCards.vueandExtrasCardcomponents for detail page card renderingBubblewithRcCounterBadgefrom rancher-componentsrelatedServiceslogic from detail page to workload modelresourceConditions,resourceEvents, and card infrastructure to base Resource classisCustomDetailOrEditis trueTechnical notes summary
cardsgetter on resource models, base_cardsprovides insight carduseResourceCardRowcomposable accepts custom keys for flexible data shapesAreas or cases that should be tested
Areas which could experience regressions
Screenshot/Video
Checklist
Admin,Standard UserandUser Base