mirror of
https://github.com/openobserve/openobserve.git
synced 2026-11-04 00:08:55 +00:00
fix(dashboards): show global variables above the tab bar (#15238)
This commit is contained in:
@@ -1,64 +1,6 @@
|
||||
---
|
||||
name: ui-architect
|
||||
description: >-
|
||||
Authoring guardrails for building ANY new frontend UI in the OpenObserve web
|
||||
app (web/) — new views, pages, panels, dialogs, feature components, or edits
|
||||
to existing ones. Enforce seven house rules the moment you write Vue/template
|
||||
markup: (1) use OPageHeader for every page/module header, (2) build UI from
|
||||
O2 library components in web/src/lib — never bare HTML controls when an O2
|
||||
equivalent exists, (3) NEVER write px — always rem, including inside Tailwind class
|
||||
arbitrary values ([320px] is banned; 1rem = 16px, so px/16 = rem and px/4 = the
|
||||
Tailwind step). px is allowed ONLY where it is the genuinely correct unit —
|
||||
hairlines, shadow/ring widths, query conditions, IntersectionObserver rootMargin,
|
||||
zero inside calc()/clamp(), user-facing copy, canvas/email consumers — and there it
|
||||
MUST carry an eslint-disable-next-line local/no-hardcoded-px with a `-- <reason>` at
|
||||
the site, never a side-file exemption (in a <style> block the directive goes inside
|
||||
the block as a CSS comment). The local/no-hardcoded-px rule runs in CI, so an
|
||||
unannotated px fails the build and cannot reach main;
|
||||
and corner radius uses only the two-tier scale rounded-default
|
||||
(4px controls) / rounded-surface (12px surfaces) / rounded-full — never
|
||||
rounded-[..] or the retired rounded-sm/md/lg/xl, (4) no scoped-CSS blocks and no
|
||||
inline style="", (5) never hardcode colors/sizes and never reach a token by raw
|
||||
var() in a component — use the modern registered --color-* design tokens through
|
||||
their utility class (bg-x/text-x), register a new --color-* token if one is
|
||||
missing; the legacy --o2-* token vocabulary is BANNED (never write var(--o2-*),
|
||||
never define one, never add a .body--dark block — migrate any --o2-* you touch to
|
||||
its --color-* equivalent); all of this is CI-enforced and fails the build,
|
||||
(6) no hardcoded user-facing text — every label, title, placeholder, and
|
||||
message comes from i18n (useI18nTyped t(), never useI18n from vue-i18n, which
|
||||
is banned) with keys added to web/src/locales/languages/en-US.json; text-carrying
|
||||
props/fields are typed I18nText and i18n keys stored as data are typed I18nKey,
|
||||
and the ONLY opt-out for a genuinely non-translatable string is raw() — never an
|
||||
eslint-disable, (7) every page is responsive — phones and tablets adapt through
|
||||
max-md:/max-lg: variants and useBreakpoint() while the laptop (≥lg) view stays
|
||||
identical; headers, toolbars and stat strips keep one row, filter toggles collapse
|
||||
to dropdowns (mobile-dropdown), secondary header actions go to #actions-overflow,
|
||||
side panels become drawers that open from their own row, row actions fold into a
|
||||
kebab, and popups fit the viewport. It also settles the recurring
|
||||
structural decisions: use OTable for any tabular data, cut text with
|
||||
OTruncatedText (full text on hover only when cut; never for secrets or text
|
||||
shown in full elsewhere; no title/OTooltip that repeats visible text), follow the
|
||||
TanStack Query layering for server data (a declared queryOptions() per read
|
||||
on its module's staleTime tier, mutationOptions() writes that invalidate,
|
||||
refresh buttons that force every read on the view, no Vuex copies), choose the
|
||||
right form container (ConfirmDialog vs ODialog vs ODrawer vs a full in-page
|
||||
view) by the weight of the interaction, and build every validated form with
|
||||
OForm + a colocated Zod schema (single-source-of-truth name-bound fields, no
|
||||
v-model/ref mirrors, automatic submit/loading, correct field-array keys).
|
||||
Trigger this whenever the user asks to create, add, build, scaffold, lay out,
|
||||
validate, or restyle any screen, component, header, table, list, dialog,
|
||||
drawer, form, field, or panel in the web frontend, asks to make any of them
|
||||
work on mobile / phone / tablet / small screens (responsive), or asks where a
|
||||
form/table/fetch should live, how to cache or refresh server data, how to validate a form, how to add a keyboard
|
||||
shortcut, how to build a new reusable/common O2 component when nothing existing
|
||||
fits (create one in web/src/lib instead of assembling divs and classes),
|
||||
whether something belongs in a dialog or a drawer, or where a new page should be
|
||||
listed in navigation — the left-rail menu, a Settings/IAM sub-menu, or a
|
||||
hover-flyout — how to register its route, and how to gate it for cloud /
|
||||
enterprise / RBAC — even if they don't mention
|
||||
these rules by name. If you are
|
||||
about to type <template>, a page title, a hex color, a px value, or
|
||||
<style scoped>, this skill applies.
|
||||
description: ALWAYS use this skill for ANY change to the OpenObserve web UI (web/) — even a single-line UI modification.
|
||||
---
|
||||
|
||||
# UI Architect — Frontend UI Guardrails (OpenObserve `web/`)
|
||||
|
||||
@@ -527,9 +527,8 @@ describe("RenderDashboardCharts", () => {
|
||||
expect(wrapper.exists()).toBe(true);
|
||||
});
|
||||
|
||||
// Variables scope the ACTIVE tab, so rendering them above the strip made them
|
||||
// read as page chrome and moved the tab bar whenever their height changed.
|
||||
it("renders the global variables strip BELOW the tab list, not above it", () => {
|
||||
// Global variables apply dashboard-wide, so the strip sits above the tab bar.
|
||||
it("renders the global variables strip ABOVE the tab list, not below it", () => {
|
||||
wrapper = createWrapper({
|
||||
showTabs: true,
|
||||
dashboardData: {
|
||||
@@ -547,7 +546,7 @@ describe("RenderDashboardCharts", () => {
|
||||
|
||||
expect(tabsAt).toBeGreaterThan(-1);
|
||||
expect(varsAt).toBeGreaterThan(-1);
|
||||
expect(tabsAt).toBeLessThan(varsAt);
|
||||
expect(varsAt).toBeLessThan(tabsAt);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -33,16 +33,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
>
|
||||
</span>
|
||||
|
||||
<!-- Tab List -->
|
||||
<TabList
|
||||
v-if="showTabs && selectedTabId !== null"
|
||||
class="mt-2"
|
||||
:dashboardData="dashboardData"
|
||||
:viewOnly="viewOnly"
|
||||
@refresh="refreshDashboard"
|
||||
/>
|
||||
|
||||
<!-- Below the tabs: these scope the ACTIVE tab, and above them the strip both read as page chrome and shifted the tab bar as its height changed per tab. -->
|
||||
<!-- Global variables sit above the tab bar: they apply dashboard-wide, not to the active tab. -->
|
||||
<VariablesValueSelector
|
||||
v-if="globalVariables.length > 0 || dashboardData?.variables?.showDynamicFilters"
|
||||
:scope="'global'"
|
||||
@@ -55,6 +46,15 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
data-test="global-variables-selector"
|
||||
/>
|
||||
|
||||
<!-- Tab List -->
|
||||
<TabList
|
||||
v-if="showTabs && selectedTabId !== null"
|
||||
class="mt-2"
|
||||
:dashboardData="dashboardData"
|
||||
:viewOnly="viewOnly"
|
||||
@refresh="refreshDashboard"
|
||||
/>
|
||||
|
||||
<!-- Tab-scoped Variables (for active tab, if using manager) -->
|
||||
<VariablesValueSelector
|
||||
v-if="variablesManager && currentTabVariables.length > 0 && selectedTabId"
|
||||
|
||||
@@ -1549,6 +1549,11 @@ describe("CuratedPageView", () => {
|
||||
(variables[0] as any).options = [{ label: "common-dev", value: "common-dev" }];
|
||||
(variables[0] as any).value = ["common-dev"];
|
||||
(variables[1] as any).value = ["argocd"];
|
||||
// Tab-scoped, as buildVariable emits them.
|
||||
for (const variable of variables as any[]) {
|
||||
variable.scope = "tabs";
|
||||
variable.tabs = ["workloads"];
|
||||
}
|
||||
// The REAL chain edge buildVariable emits — the graph is built from this
|
||||
// `$cluster` reference, so a fixture without it resets no children.
|
||||
(variables[1] as any).query_data = {
|
||||
@@ -1570,13 +1575,16 @@ describe("CuratedPageView", () => {
|
||||
.vm.$emit("variablesManagerReady", manager);
|
||||
await flushPromises();
|
||||
|
||||
manager.updateVariableValue("cluster", "global", undefined, undefined, []);
|
||||
manager.updateVariableValue("cluster", "tabs", "workloads", undefined, []);
|
||||
manager.commitAll();
|
||||
await flushPromises();
|
||||
|
||||
expect(router.currentRoute.value.query["var-cluster"]).toBeUndefined();
|
||||
const query = router.currentRoute.value.query;
|
||||
expect(query["var-cluster"]).toBeUndefined();
|
||||
expect(query["var-cluster.t.workloads"]).toBeUndefined();
|
||||
// The chain: a namespace picked under the cleared cluster must not keep filtering.
|
||||
expect(router.currentRoute.value.query["var-namespace"]).toBeUndefined();
|
||||
expect(query["var-namespace"]).toBeUndefined();
|
||||
expect(query["var-namespace.t.workloads"]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("finding 23b: an omitted cluster picker with ONE resolvable value renders the name as static text", async () => {
|
||||
@@ -1741,7 +1749,8 @@ describe("CuratedPageView", () => {
|
||||
label: "K8s Namespace",
|
||||
type: "query_values",
|
||||
multiSelect: true,
|
||||
scope: "global",
|
||||
scope: "tabs",
|
||||
tabs: ["workloads"],
|
||||
value: [],
|
||||
options: [
|
||||
{ label: "argocd", value: "argocd" },
|
||||
@@ -1800,11 +1809,11 @@ describe("CuratedPageView", () => {
|
||||
.vm.$emit("variablesManagerReady", manager);
|
||||
await flushPromises();
|
||||
|
||||
manager.updateVariableValue("namespace", "global", undefined, undefined, ["argocd"]);
|
||||
manager.updateVariableValue("namespace", "tabs", "workloads", undefined, ["argocd"]);
|
||||
await flushPromises();
|
||||
const afterFirst = committedValue(manager, "namespace");
|
||||
|
||||
manager.updateVariableValue("namespace", "global", undefined, undefined, ["argocd"]);
|
||||
manager.updateVariableValue("namespace", "tabs", "workloads", undefined, ["argocd"]);
|
||||
await flushPromises();
|
||||
|
||||
expect(committedValue(manager, "namespace")).toEqual(afterFirst);
|
||||
@@ -1824,11 +1833,11 @@ describe("CuratedPageView", () => {
|
||||
expect(committedValue(manager, "namespace")).toEqual([]);
|
||||
|
||||
// Exactly what VariablesValueSelector does on a user pick.
|
||||
manager.updateVariableValue("namespace", "global", undefined, undefined, ["argocd"]);
|
||||
manager.updateVariableValue("namespace", "tabs", "workloads", undefined, ["argocd"]);
|
||||
await flushPromises();
|
||||
|
||||
// Live state moved...
|
||||
expect(manager.getVariable("namespace", "global")?.value).toEqual(["argocd"]);
|
||||
expect(manager.getVariable("namespace", "tabs", "workloads")?.value).toEqual(["argocd"]);
|
||||
// ...and the page must have pushed it through to what panels read.
|
||||
expect(committedValue(manager, "namespace")).toEqual(["argocd"]);
|
||||
});
|
||||
@@ -1848,7 +1857,8 @@ describe("CuratedPageView", () => {
|
||||
label: "Cluster",
|
||||
type: "query_values",
|
||||
multiSelect: false,
|
||||
scope: "global",
|
||||
scope: "tabs",
|
||||
tabs: ["workloads"],
|
||||
value: "prod-eu",
|
||||
options: [
|
||||
{ label: "prod-eu", value: "prod-eu" },
|
||||
@@ -1873,7 +1883,7 @@ describe("CuratedPageView", () => {
|
||||
.vm.$emit("variablesManagerReady", manager);
|
||||
await flushPromises();
|
||||
|
||||
manager.updateVariableValue("cluster", "global", undefined, undefined, "prod-us");
|
||||
manager.updateVariableValue("cluster", "tabs", "workloads", undefined, "prod-us");
|
||||
await flushPromises();
|
||||
|
||||
const render = wrapper.findComponent({ name: "RenderDashboardCharts" });
|
||||
@@ -1882,7 +1892,7 @@ describe("CuratedPageView", () => {
|
||||
await flushPromises();
|
||||
|
||||
expect(router.currentRoute.value.query.tab).toBe(dashboardData.tabs[1].tabId);
|
||||
expect(manager.getVariable("cluster", "global")?.value).toBe("prod-us");
|
||||
expect(manager.getVariable("cluster", "tabs", "workloads")?.value).toBe("prod-us");
|
||||
});
|
||||
|
||||
it("a picker change does NOT re-resolve — only the selection moved", async () => {
|
||||
@@ -1900,7 +1910,7 @@ describe("CuratedPageView", () => {
|
||||
refreshSpy.mockClear();
|
||||
const identityBefore = captured.dashboardData;
|
||||
|
||||
manager.updateVariableValue("namespace", "global", undefined, undefined, ["ziox"]);
|
||||
manager.updateVariableValue("namespace", "tabs", "workloads", undefined, ["ziox"]);
|
||||
await flushPromises();
|
||||
|
||||
expect(refreshSpy).not.toHaveBeenCalled();
|
||||
|
||||
@@ -550,7 +550,10 @@ const onVariablesManagerReady = (manager: VariablesManager) => {
|
||||
stopCommitWatch?.();
|
||||
// Values ONLY: options and loading flags churn per fetch and would re-run panels for an untouched picker.
|
||||
stopCommitWatch = watch(
|
||||
() => manager.variablesData.global.map((variable) => variable.value),
|
||||
() =>
|
||||
Object.values(manager.variablesData.tabs).flatMap((variables) =>
|
||||
variables.map((variable) => variable.value),
|
||||
),
|
||||
() => {
|
||||
manager.commitAll();
|
||||
syncPickerUrl(manager);
|
||||
@@ -595,12 +598,16 @@ watch(
|
||||
const manager = variablesManager.value;
|
||||
if (!manager) return;
|
||||
// loadFromUrl only APPLIES the keys it finds, so a Back that drops var-cluster would strand the drilled-in value.
|
||||
const cluster = manager.getVariable("cluster", "global");
|
||||
if (cluster && query["var-cluster"] === undefined) {
|
||||
for (const [tabId, variables] of Object.entries(manager.variablesData.tabs)) {
|
||||
const cluster = variables.find((variable) => variable.name === "cluster");
|
||||
if (!cluster) continue;
|
||||
if (query["var-cluster"] !== undefined || query[`var-cluster.t.${tabId}`] !== undefined) {
|
||||
continue;
|
||||
}
|
||||
manager.updateVariableValue(
|
||||
"cluster",
|
||||
"global",
|
||||
undefined,
|
||||
"tabs",
|
||||
tabId,
|
||||
undefined,
|
||||
cluster.multiSelect ? [] : (cluster.options?.[0]?.value ?? null),
|
||||
);
|
||||
|
||||
@@ -1352,7 +1352,7 @@ describe("§5.5 buildDashboard", () => {
|
||||
expect(namespace.type).toBe("query_values");
|
||||
expect(namespace.multiSelect).toBe(true);
|
||||
expect(namespace.selectAllValueForMultiSelect).toBe("all");
|
||||
expect(namespace.scope).toBe("global");
|
||||
expect(namespace.scope).toBe("tabs");
|
||||
expect(namespace.query_data).toEqual(
|
||||
expect.objectContaining({
|
||||
stream_type: "metrics",
|
||||
@@ -2062,6 +2062,8 @@ describe("§8.2 golden parity — hosts pack vs the frozen buildHostDashboard ou
|
||||
|
||||
const manager = useVariablesManager(((key: string) => key) as never);
|
||||
await manager.initialize(built.variables.list, built);
|
||||
// Tab-scoped pickers are marked pending when their tab becomes visible.
|
||||
manager.setTabVisibility("workloads", true);
|
||||
const managed = manager.getAllVisibleVariables("workloads") as any[];
|
||||
expect(managed.length).toBeGreaterThan(0);
|
||||
|
||||
@@ -2142,7 +2144,8 @@ describe("§8.2 golden parity — hosts pack vs the frozen buildHostDashboard ou
|
||||
}
|
||||
expect(ours.queryDataKeys).toEqual(theirs.queryDataKeys);
|
||||
expect(ours.type).toBe(theirs.type);
|
||||
expect(ours.scope).toBe(theirs.scope);
|
||||
// Tab-scoped so each picker renders below the tab bar on the sections it scopes.
|
||||
expect(ours.scope).toBe("tabs");
|
||||
expect(ours.multiSelect).toBe(theirs.multiSelect);
|
||||
// The sentinel VALUE is a deliberate per-picker choice, not a shape contract:
|
||||
// `cluster` opts into "first" so the page opens on one cluster rather than a
|
||||
@@ -2473,9 +2476,7 @@ describe("the built dashboard does not depend on the selected tab", () => {
|
||||
// and would never fetch its options at all).
|
||||
it("each built picker declares the sections that scope it", () => {
|
||||
const dashboard: any = build(resolve({}));
|
||||
const byName = Object.fromEntries(
|
||||
dashboard.variables.list.map((v: any) => [v.name, v.curatedTabs]),
|
||||
);
|
||||
const byName = Object.fromEntries(dashboard.variables.list.map((v: any) => [v.name, v.tabs]));
|
||||
// Cluster renders on ALL THREE tabs: a Workloads without it answered a
|
||||
// per-cluster question with every cluster's pods, silently.
|
||||
expect(byName.cluster).toEqual(["inventory", "health", "utilization", "nodes", "workloads"]);
|
||||
@@ -2528,7 +2529,7 @@ describe("the built dashboard does not depend on the selected tab", () => {
|
||||
const expected = kubernetesPage.sections
|
||||
.filter((section) => (section.scopedBy ?? []).includes(variable.name))
|
||||
.map((section) => section.id);
|
||||
expect(variable.curatedTabs, variable.name).toEqual(expected);
|
||||
expect(variable.tabs, variable.name).toEqual(expected);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1214,17 +1214,13 @@ function buildVariable(
|
||||
return resolved ? [{ name, resolved }] : [];
|
||||
});
|
||||
|
||||
// The sections this picker applies to, declared on the variable the way the
|
||||
// dashboards model declares `tabs` — read as a reactive filter at render time
|
||||
// so switching tabs never rebuilds this object. Scope stays "global": an
|
||||
// all-sentinel variable at tab scope is skipped by setTabVisibility and would
|
||||
// never fetch its options at all.
|
||||
const curatedTabs = manifest.sections
|
||||
// Tab-scoped on the sections that declare it, so the picker renders below the tab
|
||||
// bar and each tab keeps its own selection.
|
||||
const tabs = manifest.sections
|
||||
.filter((section) => (section.scopedBy ?? []).includes(def.name))
|
||||
.map((section) => section.id);
|
||||
|
||||
return {
|
||||
curatedTabs,
|
||||
curatedPickerLabel: picker.label,
|
||||
name: def.name,
|
||||
label: def.labelKey ? "" : picker.label,
|
||||
@@ -1236,7 +1232,8 @@ function buildVariable(
|
||||
...(def.multiSelect
|
||||
? { selectAllValueForMultiSelect: def.defaultFirstValue ? "first" : "all" }
|
||||
: {}),
|
||||
scope: "global",
|
||||
scope: "tabs",
|
||||
tabs,
|
||||
omitWhenValuesEmpty: def.omitWhenValuesEmpty === true,
|
||||
// Consumed by VariablesValueSelector; stored dashboards never carry them, so they stay invisible outside curated pages.
|
||||
curatedOmitWhenValuesEmpty: def.omitWhenValuesEmpty === true,
|
||||
|
||||
Reference in New Issue
Block a user