feat: Add filter templating to custom dashboard on-click#2146
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: f4a954c The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| enableMapSet(); | ||
|
|
||
| describe('searchFilters', () => { | ||
| describe('filtersToQuery', () => { |
There was a problem hiding this comment.
filtersToQuery and its tests have been moved to common-utils
8902ef4 to
7f746b7
Compare
ca2be83 to
de0c501
Compare
de0c501 to
226d4ff
Compare
🔴 Tier 4 — CriticalTouches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD. Why this tier:
Review process: Deep review from a domain expert. Synchronous walkthrough may be required. Stats
|
PR Review✅ No critical issues found. The feature is well-scoped, well-tested, and the encoding/escaping concerns are handled correctly. A few minor, non-blocking observations:
Nice work — the escape/round-trip tests (backslashes + |
E2E Test Results✅ All tests passed • 169 passed • 3 skipped • 1187s
Tests ran across 4 shards in parallel. |
58a9399 to
d806f59
Compare
Deep ReviewScope: PR #2146 — dashboard onClick filter templates + URL 🔴 P0/P1 -- must fix
🟡 P2 -- recommended
🔵 P3 nitpicks (10)
Reviewers (8): correctness, security, maintainability, testing, kieran-typescript, adversarial, api-contract, julik-frontend-races. Testing gaps:
|
This is intentional to ensure the expressions will change when changing between selected dashboards.
This has essentially no functional impact that I can find.
The query will have loaded on app load, so the react query cache will always be hot - no user impact.
There are per-field errors. Sure the drawer is scrollable but (a) is unlikely to be longer than screen height without a ton of filters and (b) the user will just scroll to find the error if there is one
Yeah, the user will have to select a new target or the form will be invalid.
The dashboard will only render if useDashboards has already loaded, no impact, and not related to these changes.
MCP support is a followup, read the PR description, clanker
Hallucination. The ref isn't latched based on the ignoredFilterExpressions, just when the user actually dismisses the banner.
Fixed.
Added tests |
Incorrect, invalid handlebars template on a filter will be validated while submitting the form.
Existing, not related to these changes.
Existing, not related to these changes. |
…tile row click (#2321) ## Summary When a dashboard table tile is configured with an `onClick` action ([#2140](#2140), [#2141](#2141), [#2146](#2146), [#2148](#2148)), the row click destination is now discoverable before the user commits to the click. **What's new for users:** - **Hover hint.** After ~250ms of hover, a small card above the row reads `Search HyperDX Logs` or `Open dashboard "API Latency Drilldown"` (or generic `Open in search` / `Open dashboard` when the target is templated or the named source / dashboard no longer exists). - **Native browser behaviors.** The cell wrapper is now a real `<a href>`. Cmd-click opens the destination in a new tab, middle-click opens it in a new tab, right-click shows the browser context menu with "Open in New Tab" and "Copy Link Address", and the destination URL appears in the status bar on hover. - **Keyboard navigation.** Tab focuses the first clickable cell with a visible focus ring; Enter activates it. ### Screenshots Search action: hovering a row reveals the resolved source name. | Light | Dark | |---|---| |  |  | Dashboard action: hovering a row reveals the resolved dashboard name. | Light | Dark | |---|---| |  |  | **How the implementation changes:** - `packages/app/src/HDXMultiSeriesTableChart.tsx`: cell wrapper goes from `<div role="link" tabIndex={0}>` plus manual `onClick` / `onAuxClick` / `onMouseDown` / `onKeyDown` handlers to a Next.js `<Link href>` with `prefetch={false}`. The HoverCard wraps the whole `<tr>` at the row body level so the hint position stays stable as the cursor moves across cells; cell-level mounting would flicker per column. Rows whose templates fail to resolve render as a real `<button type="button">` (not `<a href="#">`) so cmd-click / middle-click / right-click "Open in New Tab" can't silently open a meaningless new tab against a `#` fragment. - `packages/app/src/components/DBTableChart.tsx`: drops the parent `onRowClick` cmd/middle-click branching; threads new `getRowAction` (or legacy `getRowSearchLink`) into Table. - `packages/common-utils/src/core/linkUrlBuilder.ts`: new `describeOnClick({ onClick, sourceNamesById, dashboardNamesById }): string` helper returns the one-line hint, with an exhaustiveness check on the OnClick discriminator. - `packages/app/src/hooks/useOnClickLinkBuilder.ts`: also builds `Map<id, name>` for sources and dashboards, computes the row-independent description once, and returns a per-row resolver of shape `{ url, description, onClickError? }`. Per-row results are memoized internally (WeakMap) so cells sharing a row don't rerun handlebars rendering. When a row's templates fail to resolve, `url` is `null` and `onClickError` fires the existing Mantine "Link error" toast on click. - Focus ring uses the shared `styles/focus.module.scss` `focusRing` style. - Cell wrappers carry `data-testid="dashboard-table-row-action"` on both branches; the e2e page object resolves by testid so future inline column links don't steal the click. - The legacy `getRowSearchLink` drilldown path (used outside dashboards, e.g. the services dashboard) renders as a plain `<Link href>` without a HoverCard so behavior there is unchanged. ## Test plan - [x] `make ci-lint` clean across all 3 workspaces. - [x] `make ci-unit` clean. New `HDXMultiSeriesTableChart` component test covers both success and failure branches end to end: anchor / button shape, `onClickError` wiring, row-level HoverCard hint visibility, legacy `getRowSearchLink` fallback. - [x] `describeOnClick` unit tests cover all 6 OnClickSchema discriminator + name-lookup combinations. - [x] `useOnClickLinkBuilder` hook tests cover: no onClick configured returns null, resolved source name in description, resolved dashboard name in description, row resolution failure encodes as `url: null` + click handler fires notification, WeakMap caching shares results across cells of the same row. - [x] `knip` clean (no new unused exports). - [x] UI verified in the dev stack (screenshots above). Status bar shows the destination URL on the success branch; failure branch (a `<button>`) shows no status bar URL and clicking fires the existing red `Link error` toast. Right-click on a success anchor shows the native context menu; cmd-click and middle-click open in a new tab. - [x] Both light and dark theme verified.
Summary
This is the third in a series of PRs adding customizable on-click / linking behaviors to dashboard tables. This PR extends the custom dashboard table onClick behavior from #2140.
This feature is behind the
NEXT_PUBLIC_IS_DASHBOARD_LINKING_ENABLED, which has been enabled in the preview environment and for local development.Scope
In this PR:
Not included yet
Future PRs will add:
Screenshots or video
Screen.Recording.2026-04-23.at.4.58.18.PM.mov
How to test locally or on Vercel
This can be tested in the preview environment
References