PR 1 review: every comment and what I'll do

MVP: fresh instance, no views, there's a noticable (+3 second) delay when navigating to home/explorer dashboard page and seeing the dashboard content. think this is caused by 5 sequental requests to /v1/configmaps/default?pagesize=100000&filter=metadata.labels[templates.rancher.io/ai-templating]%20IN%20(true)&exclude=metadata.managedFields. i could not reproduce this later on when there were views

What I found

Reproduced, and Richard's diagnosis is right. useViewSet.load() keeps retrying, 500ms apart, up to 5 reads, whenever it finds nothing. Stubbed reads answer instantly, so the 2 seconds below are only the waiting. Real reads add their own time, which fits Richard's 3+.

It's also wider than a fresh install. Anyone who can't read the ConfigMaps never finds anything, so they wait those 2 seconds on every visit to the Home or a cluster dashboard.

3 in-app navigations per case, on the base preview and the master preview. The 2-second wait with 0 reads is the retry loop waiting for a read it skips.

What I'll do

The loop's comment gives two reasons for retrying: the user may not be known yet, and the ConfigMaps may not exist yet. Asking for the user first solves the first. The second went away with the seeding code. So: get the user, read once, and share that read with the onEnter hook, which is why even stored views make 2 reads today. A failed read leaves the stock page showing, and the next navigation reads again.

This changes my plan. I said "retry only if the read failed", but a 403 is a failed read, so non-admins would still wait. No retry at all is simpler and fixes both.

To check it, I'll re-run this table, and open the Home straight after login to make sure views still appear.

MVP: Delta between new Cluster Explorer Dashboard components and stock Cluster Explorer Dashboard components (they should use the same components, with the table widget wrapping the dashboard one)

What I found

I told you this was the resource cards. The cards are the biggest part, but comparing them side by side shows five differences. Same cluster and window size. The numbers on the shots match the table under them.

What I'll do

Fix all five, bump the root @rancher/shell to 3.0.15-rc.4 (an RC until 3.0.15 ships), and rebuild the previews on it. Then I'll compare again on the same shell and list anything left, including the Alerts and metrics tabs on a cluster that really has monitoring.

What this won't fix: the cluster header and component status stay redrawn, because stock has them inline in its page. Truly sharing them needs a shell PR that pulls them out. I'll ask Richard whether they want that.

MVP: Hide views bar (with tabs) shortcut is currently ctrl + shift + v which clashes with unformatted paste

What I found

Shift plus a letter is already how the shell does this: its page templates bind Shift+T for the theme and Shift+L for the locale, and detail pages and tables use C, D, E, Y, J and K. V isn't taken, in the shell or in this package. The shell's shortcut plugin stays out of the way while focus is in an input, textarea, select or editor, so typing a capital V in a form is safe.

What I'll do

Set the shortcut to ['shift', 'v'] on both platforms and update the comment at index.ts:63-66. I'll press it on the preview, on a page and inside a text field.

few references to the old extension name here

1#!/usr/bin/env node
2/* eslint-disable no-console */
3// Regenerate pkg/ai-templating/install/manifests.ts from the YAML sources, so the extension can
4// embed and auto-install its cluster dependencies (see pkg/ai-templating/install/ensure.ts).
5//
6//   node scripts/gen-manifests.mjs

What I found

It regenerates pkg/ai-templating/install/manifests.ts. That folder no longer exists, and nothing in the repo calls the script.

What I'll do

Delete scripts/gen-manifests.mjs.

is this still needed?

1#!/usr/bin/env node
2/* eslint-disable no-console */
3// Regenerate pkg/ai-templating/install/manifests.ts from the YAML sources, so the extension can
4// embed and auto-install its cluster dependencies (see pkg/ai-templating/install/ensure.ts).

What I found

No, for the same reason as thread 1.

What I'll do

Same delete as thread 1.

same with this one, still needed?

1#!/usr/bin/env node
2/* eslint-disable no-console */
3// One-shot: convert the CustomView/HomeTemplate example CRs into labeled ConfigMaps (the new
4// storage). Overwrites the example files in place.

What I found

It was a one-shot that converted old example CRs into ConfigMaps. Those examples are gone, and nothing calls it.

What I'll do

Delete scripts/crs-to-configmaps.mjs. That empties scripts/.

think this is called out in template, but creating review comment just for tracking. name needs updating

1{
2  "name": "rancher-ai-template",
3  "version": "0.1.0",
4  "private": false,
5  "engines": {

What I found

Still rancher-ai-template, left over from the exact copy. Once the scripts go, the only other old name is the ai-templating label and its test. That label stays on purpose, because it is on every stored ConfigMap.

What I'll do

Set name to ui-configurable-views. The extension's own package stays configurable-views.

the prime only flag goes in here somewhere

3  "description": "configurable-views plugin",
4  "version": "0.1.0",
5  "private": false,
6  "rancher": {
7    "annotations": {
8      "catalog.cattle.io/rancher-version": ">= 2.10.0",
9      "catalog.cattle.io/ui-extensions-version": ">= 3.0.0 < 4.0.0"

What I found

The shell reads catalog.cattle.io/prime-only and refuses the extension on a non-Prime Rancher (plugins.error.primeOnly). The shell's helm patch script copies rancher.annotations from this file into the chart.

What I'll do

Add "catalog.cattle.io/prime-only": "true" under rancher.annotations. This also closes the "Make extension prime only" MVP. Baked-in previews skip this check, so only a real install proves it.

we can bump this up to 2.16

5  "private": false,
6  "rancher": {
7    "annotations": {
8      "catalog.cattle.io/rancher-version": ">= 2.10.0",
9      "catalog.cattle.io/ui-extensions-version": ">= 3.0.0 < 4.0.0"
10    }
11  },

What I found

It says >= 2.10.0. The shell turns any version with a dash into x.y.999 before comparing, so 2.16 RCs and head builds pass >= 2.16.0.

What I'll do

Set catalog.cattle.io/rancher-version to >= 2.16.0.

not sure i get this. we're redirecting from c-cluster-"configurable-views"-x to "configurableviews"-c-cluster-x? does anything use c-cluster-configurable-views?

1import type { RouteRecordRaw } from 'vue-router';
2import { ROUTE_SETTINGS, ROUTE_VIEWS } from '../templating/template-engine';
3 
4// The product's own pages are routed by the Product API (see product.ts). These are only where they
5// used to be, so an old link or bookmark still lands. They sit under the 'default' layout - named, as
6// the shell asks of an extension's routes.
7const LAYOUT = 'default';

What I found

Nothing does. They send the old prototype paths (/c/:cluster/configurable-views, /views, /layouts) to the product's pages, which the Product API routes by itself. ROUTE_SETTINGS and ROUTE_VIEWS are only used here.

What I'll do

Delete routing/index.ts, the plugin.addRoutes(routing) call and the two constants. An old prototype bookmark stops working, which is fine for something unreleased.

Draft reply

orphaned comment?

268  return list.filter((entry) => textOf(entry).toLowerCase().includes(needle));
269}
270 
271/**
272 * The resources offered in a widget's "Resource" picker. Any type Rancher knows can be typed in,
273 * but these are the ones worth suggesting.
274 */

What I found

Yes. It describes a Resource picker list that no longer exists, and sits right above the doc block of PAGINATION_CONTEXT.

What I'll do

Delete lines 271-274.

could use clusterUrl

112  const res = await store.dispatch('management/findPage', {
113    type: resource,
114    opt:  {
115      url:        `/k8s/clusters/${ encodeURIComponent(cluster) }/v1/${ resource }`,
116      transient:  true,
117      watch:      false,
118      pagination: whole ? {

What I found

clusterUrl() is at line 173 of the same file. fetchClusterPage (115) and fetchClusterSchemas (67) both build that URL by hand.

What I'll do

Use clusterUrl() in both.

there's overlap between fetchClusterPage and fetchClusterPage, can one use the other?

100  serverPaged: boolean;
101}
102 
103export async function fetchClusterPage(store: Store<unknown>, {
104  resource, cluster, page = 1, pageSize = 10, sort = [], whole = false, cap = 500
105}: ClusterPageArgs): Promise<ClusterPage> {
106  if (!resource || !cluster) {

What I found

The comment names fetchClusterPage twice. I read the second as fetchClusterRows (line 359), which makes the same findPage call and only adds filters.

What I'll do

Give fetchClusterPage an optional filters argument. fetchClusterRows then calls it with whole: true and returns rows and truncated.

we have this one already - shell/config/types.d.ts CONFIG_MAP

18 
19type Getters = Store<unknown>['getters'];
20 
21const CONFIGMAP = 'configmap';
22const NAMESPACE = 'default';
23 
24// Every stored ConfigMap carries the marker; the type label tells them apart.

What I found

CONFIG_MAP in @shell/config/types has the same value.

What I'll do

Drop the local CONFIGMAP and import it.

up to you, but for all references to vuex Store you could use shell/types/store/vuex.d.ts VuexStore. That might make it easier if we ever type that store properly

16import { migrateViewSet } from './view-model';
17import type { ViewSet } from './types';
18 
19type Getters = Store<unknown>['getters'];
20 
21const CONFIGMAP = 'configmap';
22const NAMESPACE = 'default';

What I found

The comment says it's optional, but it's cheap. VuexStore is in published shell 3.0.14 and later, and there are 24 non-test references to Store<...>.

What I'll do

Replace them. If the type check finds a place where VuexStore is too narrow, I'll list it rather than cast around it.

can home and if used somewhere cluster dashboard keys be consts somewhere

83 * like the dashboard, the view leaves them out wherever a cluster does not have them.
84 */
85export function stockWidgets(page: PageKey, t: Translate): WidgetNode[] {
86  if (page === 'home') {
87    const { gap, pad } = stockView(page);
88 
89    // The banner runs edge to edge over the view's inset, as the Home's does; the panel below it

What I found

'home' means three things in this package: a page key, a links source and the pagination context. Only the page key and 'clusterDashboard' are what this comment is about.

What I'll do

Add PAGE_HOME and PAGE_CLUSTER_DASHBOARD next to PageKey, and derive the type from them. The other two meanings of 'home' stay. Stored keys don't change.

terminology wise should this name contain or be Page? i think it's then something like... - Page - views (which are the page's tabs) - view (a page tab) - widgets - table - overview - tabs - widgets

157export type View = LayoutView | StockView;
158 
159/** What one scope - the organization, or one person - has saved for a page: its views, and which opens first. */
160export interface ViewSet {
161  views: View[];
162  defaultViewId?: string;
163  /**

What I found

It holds what one scope, the organization or one person, saved for a page: its views, which one opens first, and the tab order. Plain Page doesn't fit, because PageConfig already holds all the scopes of a page. The type name isn't stored anywhere.

What I'll do

Rename ViewSet, useViewSet and migrateViewSet to PageViews, usePageViews and migratePageViews. That's 63 references in 12 files, tests included. It's mechanical, so it gets its own commit.

Draft reply

can this use shell/plugins/steve/schema.d.ts (would need updating)

2// cluster's. Pure functions - no Vue, no store.
3 
4/** A schema as Steve serves it - the parts the Resource picker reads. */
5export interface TypeSchema {
6  id: string;
7  collectionMethods?: string[];
8  attributes?: {

What I found

Schema in shell/plugins/steve/schema.d.ts has id and attributes, but not collectionMethods, attributes.kind or attributes.group, which the Resource picker reads. Richard already expects it needs updating.

What I'll do

Import Schema and extend it locally with those three fields. The shell type gets them in the same dashboard PR as thread 28, then the local part goes.

Draft reply

there's a few dangling comments above functions instead of jsdocs across quite a few files. worth getting ai to do a convert to jsdoc run

116    }
117  }
118 
119  // Write the draft to your account.
120  async function save({ keepEditing = false } = {}): Promise<void> {
121    const draft = working.value;
122 

What I found

A script finds 55 // comments directly above a function, in 21 files. useGridEditing and useEditSession have the most, 7 each. There's also a doc block in the wrong place at widget-data.ts:188.

What I'll do

Convert them to /** */ and move the misplaced one. Section-divider comments stay. Comment-only commit, so it's easy to review.

something feels off here for items in this hardcoded list we basically don't do server-side pagination, given view-models.ts READ_FROM_A_CLUSTER.includes(out.resource)? unless that's set directly to cluster in widgetsettingsmodal? i gave this a test with pods and my CR and in both dashboards we're not doing SSP

300    out.ownViews = true;
301  }
302  if (out.kind === TABLE_KIND) {
303    out.fromCluster = typeof w.fromCluster === 'boolean' ? w.fromCluster : READ_FROM_A_CLUSTER.includes(out.resource);
304  }
305 
306  // `subtitle` and `image` are banner-only extras; keep them only when set so stored specs stay small.

What I found

READ_FROM_A_CLUSTER only fills in fromCluster for tables saved before that setting existed. All 5 tables stored on mf set it explicitly, so nothing depends on it. It also isn't why Richard saw no SSP. See two ways a table reads.

What I'll do

Remove the list, so a table with no fromCluster reads from Rancher, and update the test at view-model.test.ts:151. In PR 2, the agent's prompt has to start setting fromCluster, because it doesn't mention it today.

Draft reply

You should be able to make use of the LinkDetail getCustomDetailLink to - check for a cluster in the url - swap out the target cluster This means the name will work, but also for other usages like namespace and pod node links

83 * wrong cluster - the stock Home's own cluster table drops it for the same reason.
84 */
85function withoutDetailLink(header: TableColumn): TableColumn {
86  if (header?.formatter !== 'LinkDetail') {
87    return header;
88  }
89 

What I found

Richard is right, and there's a live bug next to it. I put a table of local's pods on created-dashboard-2's dashboard, in my browser only. Name has no link, because withoutDetailLink removes it. Namespace links to /c/c-m-bmsk94st/explorer/namespace/static-preview and Node to /c/c-m-bmsk94st/explorer/node/ip-10-0-0-253, both into the wrong cluster. On the Home those columns are plain text.

The catch: getCustomDetailLink only reaches formatters that declare it. Name uses LinkDetail, which does. Namespace uses LinkName and a pod's Node uses LinkNamePopover. Neither does, and both read the open cluster from the store. A table-level function also gets only the row, so it can't tell columns apart. My plan said this would only miss the node link. It misses the namespace link too.

What I'll do

On the open cluster the stock formatters are already right, so they stay. For a table of any other cluster, headersFor swaps each link column for LinkDetail with its own getCustomDetailLink in formatterOpts, which the table binds after its own prop, so it wins. Name uses row.detailLocation, reference columns such as a PVC's claim use their reference, and Namespace and Node build the route from their type and value. All of them get the widget's cluster and the explorer product. Tables reading from Rancher treat local as their cluster. In those tables the Node popover goes away, because it fetches from the open cluster. I'll check pods, PVCs, a CRD and a Rancher type such as clusters on the preview.

Draft reply

will this persist over log out / in? normally it would be yes, but for extensions do we unload on log out? might be safer as a cache in the core or management stores

151// request answers both: an answer is kept for a few seconds, and a request still in flight is
152// shared rather than repeated. Short on purpose - this is de-duplication, not a cache to go stale.
153const SHARED_MS = 5000;
154const shared = new Map<string, { at: number; promise: Promise<unknown> }>();
155 
156function once<T>(key: string, load: () => Promise<T>): Promise<T> {
157  const hit = shared.get(key);

What I found

It depends on the install. From reading the shell: auth/logout removes non-builtin extensions and login loads the script again, so a real extension starts fresh. Baked-in builds, like our previews, keep the module, so entries can survive for up to their 5-second window. An onLogout hook wouldn't help a real extension, because it's removed before that hook runs. monitoring in useWidgetPresence has the same shape with a 60-second window. I haven't tested this with a real extension.

What I'll do

Add the user's id to the cache keys and delete expired entries on each read, in both places.

Draft reply

not sure i've got the right input, but the text isn't vertically centred

Richard, a few minutes later:

ah, not here, in pkg/configurable-views/components/EditViewSidebar.vue. the one in view --> drop down --> rename view is fine

515          :data-testid="`configurable-views-tab-${ view.id }`"
516          @mousedown="startTabDrag(view, $event)"
517        >
518          <input
519            v-if="editing && view.id === activeId"
520            ref="nameInput"
521            class="vbar__name"

What I found

My first guess, the missing type="text", was wrong: adding it changes no computed style. Richard's screenshot is the bar's edit input (30px box, text about 1px low), and my render matches it. The rename input Richard calls fine has the same class and measures the same, so I can't reproduce a difference between them. It's the font sitting a little low, not a broken rule.

Positive means below centre. CSS pixels, from a 3x screenshot.

What I'll do

padding-bottom: 1px on .vbar__name and .evs__field. I tried it with an injected style in all three engines, which gave the last column.

Draft reply

i think we use RcButton now everywhere instead of buttons with classes

507        >
508          {{ t('configurableViews.sidebar.publish') }}
509        </button>
510        <button
511          class="btn btn-sm role-link evs__wide evs__danger"
512          @click="$emit('delete')"
513        >

What I found

There are 30 raw <button>s in 7 files. I told you 31, but one was the word in a code comment. Everything outlined red in this shot is a raw button.

What I'll do

Convert 29, then check each group on the preview in both themes.

think _most_ (all?) of the buttons can be RcButtons

544            :distance="4"
545            :aria-label="t('configurableViews.bar.moreActions')"
546          >
547            <button
548              :ref="(el) => keepRef(tabButtons, view.id, el as HTMLElement | null)"
549              type="button"
550              role="tab"

What I found

This line is the bar's tab button. It can convert: a role="tab" passed from here overrides RcButton's own role, and RcButton exposes focus() for the arrow keys.

What I'll do

Convert it with thread 21. If the tab's look or keyboard handling suffers, I'll keep this one and say why in the reply.

Should this use the Tabbed and Tab components?

89</script>
90 
91<template>
92  <div class="wtabs">
93    <h3
94      v-if="widget.title"
95      class="wtabs__title"

What I found

The comment in the file gives two reasons for not using Tabbed. The URL hash one is out of date, because it has useHash (in 3.0.14 too). The other is that its tabs are child components, while these are data that change during editing. A v-for of Tab should handle that, but I haven't tried it. The hard part is dragging a widget over a tab to open it, which Tabbed doesn't do. The strip already copies its look, so users shouldn't notice the swap.

What I'll do

Tabbed with :use-hash="false", one Tab per item (name, label, weight, and badge for the count while editing), content behind a v-if so hidden tabs don't fetch, and a drag listener on the strip for open-on-hover. If that breaks dragging, I'll keep the strip and tell you why before replying.

can we flip this somehow Widgets contain the metadata to do stuff like showing data source things (which is currently the case) but also supply a vue component to configure whatever settings it needs? that keeps components as owners of what they show and their configuration... and means extensions in the future can easily add widgets

439          </template>
440        </template>
441 
442        <template v-if="draft.kind === 'banner'">
443          <label class="wsm__label">{{ t('configurableViews.widgetSettings.subtitle') }}</label>
444          <input
445            v-model="draft.subtitle"

What I found

Richard is right about the shape. A new widget kind touches three places today: a v-if block in this dialog (lines 342, 382, 406, 442, 456, 475), RENDERERS in WidgetHost.vue and CLUSTER_WIDGETS in widget-catalog.ts. An extension can't add to any of them.

What I'll do

Each kind declares its component, its settings component and its metadata in one entry. The dialog keeps only what they all share. It's the biggest change here (the dialog is 760 lines, 950 in the demo), so I'd keep it out of this PR to avoid holding up the base. Tell me if you want it in PR 1 instead.

Draft reply

something feels off here i created a CRD in a downstream cluster. i then created a home page list targeting that cluster and using the CRD. on refresh the page correctly loads the table and shows results from that cluster. according to this though it should fail to load given it's not in management, current cluster (cluster) or cluster?

47 
48  const stores = ['management', getters['currentStore'](type), 'cluster'];
49 
50  return stores.find((store) => store && getters[`${ store }/schemaFor`]?.(type)) || 'management';
51}
52 
53/** Every cluster the user can see, as picker options, by the name a person would recognise. */

What I found

storeForType only picks a store to look a schema up in, and falls back to management. For a table reading from a cluster, nothing depends on that: the schema comes from that cluster's /v1/schemas, and the rows from a findPage given that cluster's URL.

What I'll do

No change.

Draft reply

i'm not 100% sure but this could be one of the reasons not many lists use server-side pagination The pagination context is set to home... which is only set for a couple of the cluster types.

192      :schema="schema"
193      :headers="headers"
194      :pagination-headers="pagedHeaders"
195      :context="PAGINATION_CONTEXT"
196      :override-in-store="inStore"
197      :table-views="true"
198      :table-view-tabs="viewTabs"

What I found

Richard is right. See two ways a table reads.

What I'll do

No change now. This is the "Later: Integrate SSP" item.

Draft reply

this is on shell/models/steve-schema.ts canList it doesn't to the get, but it's the common place we check if we can list

27 * Can a list of it be read? A Kubernetes kind whose collection answers a GET, and, where the schema
28 * says which verbs it allows, one of them is `list` - some, like a kubeconfig, can only be made.
29 */
30export function isListable(schema: TypeSchema): boolean {
31  const verbs = schema.attributes?.verbs;
32 
33  return !!schema.attributes?.kind &&

What I found

canList is a getter on the schema model, and its helper canSchema isn't exported. These schemas are raw JSON from another cluster, so they aren't models. canSchema asks for a collection link and the verb. isListable asks for a kind, a GET collection method and the verb only if verbs are present.

What I'll do

Keep isListable, but make it ask what canList asks as well: the collection link, and always the verb. This is the one small code change in this group.

Draft reply

feels like one for shell/utils/object.js

132 
133type Loose = Record<string, unknown>;
134 
135function isObject(v: unknown): v is Loose {
136  return !!v && typeof v === 'object' && !Array.isArray(v);
137}
138 

What I found

shell/utils/object.js doesn't export one today. Moving it needs a dashboard PR and a shell release.

What I'll do

Keep it local for now. Do it in one dashboard PR with the Schema fields from thread 15. Two other shell changes would help later: unhide the feature flag, and let useResourceStateSummaries take a cluster.

Draft reply

this will need to live in a place where all users can access it. i think we can shift it to be per user, and in userpreferences. alternatively we can just rely on - global feature flag - temporary runtime disable (if the user then wants to disable feature flag via ui)

155 
156/** data.enabled === 'false' turns the feature off. Absent, or anything else, means on. */
157export function isTemplatingEnabled(getters: Getters): boolean {
158  return cmNamed(getters, CONFIG_NAME)?.data?.enabled !== 'false';
159}
160 
161/** The URL's own switch: `?confviews=false`. */

What I found

Richard is right. isTemplatingEnabled reads templating-config in default. A user who can't read it gets undefined, which counts as on, so turning it off never reaches them.

What I'll do

See Decision 1. I recommend Richard's alternative: the global flag plus a temporary runtime off.

Draft reply

Bringing across https://github.com/rancher/dashboard/pull/19293#discussion_r4123918310 Tracked elsewhere for discussion. We'll need to make it clear these tables do not update (given every other visually same table does update). Ideally we allow the user to manually refresh

109    };
110  }
111 
112  const res = await store.dispatch('management/findPage', {
113    type: resource,
114    opt:  {
115      url:        `/k8s/clusters/${ encodeURIComponent(cluster) }/v1/${ resource }`,

What I found

A table reading from a cluster is read once (transient: true, watch: false). On dashboard PR 19293 I answered that these are a snapshot read when the view opens. Richard is right that it needs to show, because they look like every other table.

What I'll do

See Decision 2, which has a mock-up.

Draft reply

Tracking here UX: Discussion on these pages required

12 
13// Its two pages. The one on top is where the product opens, so Settings - the kill switch - leads:
14// always reachable, including when the feature is off.
15export const pages: ProductChildCustomPage[] = [
16  {
17    name:      PAGE_SETTINGS,
18    labelKey:  'configurableViews.settings.label',

What I found

The product has two pages. Settings holds only the switch from thread 29. Configurable Views lists everything saved per page, with Edit YAML. If Decision 1 goes as I recommend, Settings leaves the base, which answers half of Richard's "is there a need for the global product page". The other page is useful to us for debugging. Whether anyone else needs it is the UX question.

What I'll do

No code until UX answers.

minor, one for UX. should this be a user preference?

1import { ref } from 'vue';
2 
3/** Per browser: whether the bar shows is a matter of taste, not something to sync between people. */
4const STORAGE_KEY = 'configurable-views-bar-visible';
5 
6function read(): boolean {

What I found

It's in localStorage today, per browser, on purpose. A preference would follow the person between machines. It's a small change either way.

What I'll do

No change until UX answers.