From 5e8a83e11ac1592f4dc953c7d86d461b2d2a3f8b Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 15:47:43 +0800 Subject: [PATCH 01/12] feat(studio): add explorer_banner_exposed impression event (#49747) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _Requested by **Pam Chia** · [Slack thread](https://supabase.slack.com/archives/C076KTY11DF/p1788139328573799?thread_ts=1788139328.573799&cid=C076KTY11DF)_ ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Feature (telemetry). Adds one PostHog event. Linear issue: [GROWTH-1154](https://linear.app/supabase/issue/GROWTH-1154/telemetry-explorer-feature-preview-banner-has-no-exposure-event-so) ## What is the current behavior? The Explorer feature preview banner emits `explorer_banner_dismiss_button_clicked` and `explorer_banner_cta_button_clicked` (both from `apps/studio/components/ui/BannerStack/Banners/BannerExplorer.tsx`, shipped in #49606), and nothing else. With no impression event there is no denominator, so no click-through or dismiss rate can be reported. ## What is the new behavior? `explorer_banner_exposed` fires when the banner content is rendered, at most once per page load. The event is declared in `packages/common/telemetry-constants.ts` next to the two existing Explorer banner events and added to the `TelemetryEvent` union, following the existing `*_exposed` family. It carries no custom properties; `project` and `organization` groups are attached by `apps/studio/lib/telemetry/track.ts`. **Verification:** - `prettier --check` on both changed files: passing - `tsc --noEmit` in `packages/common`, which covers the new event interface and the `TelemetryEvent` union: passing - Studio-scoped lint, typecheck, and tests: green on CI - Browser-tested on the studio-staging preview (Playwright): the exposure event fires exactly once per page load (201 on the wire), does not re-fire on client-side navigation or banner hover within the same page load, fires again after a full reload, and does not fire after dismissal; the CTA and dismiss click events are unchanged and carry the `project`/`organization` groups **Out of scope:** - Pre-consent drops: every telemetry event waits for consent, so this event degrades the same way the rest of the `*_exposed` family does (transient, recovers on the next page load). A family-wide fix is a separate issue. - Mirroring the `explorer` flag state into event properties: redundant once exposure exists. - The CTA handler not dismissing the banner: raised separately, both click handlers untouched. - [GROWTH-1153](https://linear.app/supabase/issue/GROWTH-1153/telemetry-advisorcategory-omitted-for-health-lints-on-two-of-five) and its draft PR #49746: separate issue, no overlap. --- _Generated by [Claude Code](https://claude.ai/code/session_01Xwj2SotnaHByjbTfqF4Kdm); reworked per Pam's review._ --------- Co-authored-by: Claude Co-authored-by: Pamela Chia --- .../ui/BannerStack/Banners/BannerExplorer.tsx | 10 ++++++++++ packages/common/telemetry-constants.ts | 14 ++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/apps/studio/components/ui/BannerStack/Banners/BannerExplorer.tsx b/apps/studio/components/ui/BannerStack/Banners/BannerExplorer.tsx index 78120cc7b397b..70979ff5054f0 100644 --- a/apps/studio/components/ui/BannerStack/Banners/BannerExplorer.tsx +++ b/apps/studio/components/ui/BannerStack/Banners/BannerExplorer.tsx @@ -1,5 +1,6 @@ import { LOCAL_STORAGE_KEYS } from 'common' import { AnimatePresence, motion } from 'framer-motion' +import { useEffect } from 'react' import { Badge, Button } from 'ui' import { BannerCard } from '../BannerCard' @@ -8,6 +9,8 @@ import { useFeaturePreviewModal } from '@/components/interfaces/App/FeaturePrevi import { useLocalStorageQuery } from '@/hooks/misc/useLocalStorage' import { useTrack } from '@/lib/telemetry/track' +let hasTrackedExposureThisPageLoad = false + export const BannerExplorer = () => { const track = useTrack() const { dismissBanner } = useBannerStack() @@ -18,6 +21,13 @@ export const BannerExplorer = () => { false ) + useEffect(() => { + if (hasTrackedExposureThisPageLoad) return + hasTrackedExposureThisPageLoad = true + + track('explorer_banner_exposed') + }, [track]) + return ( { diff --git a/packages/common/telemetry-constants.ts b/packages/common/telemetry-constants.ts index bafb97da93703..a164992611f51 100644 --- a/packages/common/telemetry-constants.ts +++ b/packages/common/telemetry-constants.ts @@ -1531,6 +1531,19 @@ export interface DatabaseConnectionsBannerCtaButtonClickedEvent { groups: TelemetryGroups } +/** + * The Explorer feature preview banner was rendered in studio project pages, fired at most once + * per page load. Acts as the denominator for the banner's dismiss and CTA rates; dedupe per + * session or per user at query time. + * + * @group Events + * @source studio + */ +export interface ExplorerBannerExposedEvent { + action: 'explorer_banner_exposed' + groups: TelemetryGroups +} + /** * User clicked the dismiss button on the Explorer feature preview banner in studio project pages. * @@ -3834,6 +3847,7 @@ export type TelemetryEvent = | DatabaseConnectionsBlockerViewClickedEvent | DatabaseConnectionsBannerDismissButtonClickedEvent | DatabaseConnectionsBannerCtaButtonClickedEvent + | ExplorerBannerExposedEvent | ExplorerBannerDismissButtonClickedEvent | ExplorerBannerCtaButtonClickedEvent | SessionTerminateButtonClickedEvent From 5b01b5a9c7a4b9a916a58095fb308f1dad7ff754 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Mon, 31 Aug 2026 15:48:01 +0800 Subject: [PATCH 02/12] fix(studio): report advisorCategory consistently across advisor telemetry surfaces (#49746) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _Requested by **Pam Chia** · [Slack thread](https://supabase.slack.com/archives/C076KTY11DF/p1788139328573799?thread_ts=1788139328.573799&cid=C076KTY11DF)_ ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Bug fix (telemetry correctness). No user-visible change. ## What is the current behavior? Linear: [GROWTH-1153](https://linear.app/supabase/issue/GROWTH-1153/telemetry-advisorcategory-omitted-for-health-lints-on-two-of-five) **Before:** five surfaces emit the optional `advisorCategory` property on `advisor_detail_opened` and `advisor_assistant_button_clicked`, and they disagree about how to derive it. Three pass the lint's category straight through as `categories[0]`. Two compute it with a hardcoded ladder — `categories.includes('SECURITY') ? 'SECURITY' : categories.includes('PERFORMANCE') ? 'PERFORMANCE' : undefined` — which predates the `HEALTH` category and falls through to `undefined` for anything it does not name. Because the property is optional, those two surfaces ship the event with `advisorCategory` silently absent: no type error, no runtime error, just a hole in the data. A reader querying a category breakdown of either event gets numbers that depend on which surface the user happened to click, and `HEALTH` is under-counted. The split is clearest in `AdvisorSection.tsx`, where a single advisor card emits both events — the card click through the ladder (L83) and the Assistant button through the pass-through (L206) — so one card can report two different categories for the same lint. The cause is that `AdvisorCategory` in `packages/common/telemetry-constants.ts` is schema-derived: ```ts type AdvisorCategory = components['schemas']['GetProjectLintsResponse'][number]['categories'][number] ``` The API-types regeneration in supabase/supabase #49646 (merged 2026-08-27, `26e89b36c349893540f8efbd45613921be0a4d18`) widened `categories` from `('PERFORMANCE' | 'SECURITY')[]` to `('PERFORMANCE' | 'SECURITY' | 'HEALTH')[]`. `AdvisorCategory` picked up the third value incidentally and the two ladders were never updated — a union widening is invisible to a hardcoded ladder, so nothing broke loudly. | Event | Surface | HEALTH behavior before | | --- | --- | --- | | `advisor_detail_opened` | `apps/studio/components/ui/AdvisorPanel/AdvisorPanel.tsx` (L203) | ladder → property absent | | `advisor_detail_opened` | `apps/studio/components/interfaces/ProjectHome/AdvisorSection.tsx` (L83) | ladder → property absent | | `advisor_detail_opened` | `apps/studio/components/interfaces/Linter/LinterDataGrid.tsx` (L163) | pass-through → `'HEALTH'` | | `advisor_assistant_button_clicked` | `apps/studio/components/interfaces/Linter/LintDetail.tsx` (L38) | pass-through → `'HEALTH'` | | `advisor_assistant_button_clicked` | `apps/studio/components/interfaces/ProjectHome/AdvisorSection.tsx` (L206) | pass-through → `'HEALTH'` | The two `advisorCategory` property doc comments in `telemetry-constants.ts` (L2949, L2980) also still read "Category of the advisor (SECURITY or PERFORMANCE)", which the widening made false. ## What is the new behavior? **After:** all five surfaces derive `advisorCategory` the same way, so a category breakdown of these two events is consistent regardless of which surface produced the event, and `HEALTH` is reported wherever it can occur. The two ladder sites now read `item.original.categories[0]`, matching the three sites that already did. The `signal` branch (which reports `'SECURITY'`) and the `notification` branch (`undefined`) of those two expressions are unchanged, so nothing about non-lint advisor items moves. The stale parenthetical is cut from both doc comments. Net diff is 3 files, -12/+4 lines. No behavior change outside the value of one optional telemetry property. ## Additional context **How.** The fix is the pass-through, not an extended ladder. Per the two options considered: 1. **No lint carries more than one category in practice.** Every lint fixture in `apps/studio` uses a single-element array (`['SECURITY']`, `['PERFORMANCE']`). The API type permits a multi-element array, but nothing in the repo produces one, so the ladder's SECURITY-over-PERFORMANCE priority is not load-bearing. 2. **The advisors UI already treats the first element as canonical** — `LinterDataGrid.tsx` L196 renders ``. 3. **Extending the ladder would not actually produce agreement.** In the one reachable multi-category case, a ladder with a `HEALTH` branch appended still reports the higher-priority category while the three pass-through sites report `categories[0]`. Only `categories[0]` makes all five agree, which is the point of the change. **Reviewers should look at this first — how much data is actually affected.** Narrower than the headline suggests, and worth stating precisely. Every surface feeding these events filters lints upstream by category, and all three filters still admit only `SECURITY` or `PERFORMANCE`: - `AdvisorPanel.utils.ts` `createAdvisorLintItems` drops any lint that resolves to no tab (`if (!tab) return null`), and it is the item source for **both** ladder surfaces - `pages/project/[ref]/advisors/security.tsx` filters `categories.includes('SECURITY')` - `pages/project/[ref]/advisors/performance.tsx` filters `categories.includes('PERFORMANCE')` So a HEALTH-**only** lint is not surfaced anywhere in Studio today and cannot currently reach any of the five emit sites. The divergence reachable today is a lint carrying `HEALTH` alongside another category: it passes the filters, and then the ladder sites and the pass-through sites disagree. The HEALTH-only omission is latent, and becomes live data loss the moment HEALTH lints are surfaced — presumably the point of the API adding the category. Practical consequence: **no backfill or historical-data caveat is needed**, because no HEALTH-only event was ever emitted. This is a correctness fix that gets the emit surfaces right ahead of the category being shown, not a response to an active data incident. **How it was tested.** Honest caveat up front: `pnpm install` cannot complete in this sandbox, so the Studio-scoped checks could not be run here. `apps/studio` depends on `@std/path` → `npm:@jsr/std__path`, and the JSR registry is network-blocked in this environment (`GET https://npm.jsr.io/~/11/@jsr/std__path/1.0.8.tgz` → `403`, both direct and proxied; `registry.npmjs.org` returns `200`, so it is JSR specifically). CI on this PR is the real signal for Studio lint, typecheck, and tests. What did run clean: - `prettier --config prettier.config.mjs --check` on all three changed files — clean - `tsc --noEmit` in `packages/common` (installed via `pnpm install --filter=common...`) — clean, and `--listFiles` confirms it genuinely covers both `telemetry-constants.ts` and the widened `packages/api-types/types/platform.d.ts` - the changed expression typechecked in a standalone harness against the real generated `components['schemas']['GetProjectLintsResponse']`, confirming `categories[0]` is assignable to `AdvisorCategory | undefined` — with a negative control that correctly errored (`Type '"HEALTH"' is not assignable to type '"PERFORMANCE" | "SECURITY" | undefined'`) to prove the harness had teeth No tests are added. There is no existing test coverage of `handleItemClick` / `handleCardClick` in either ladder component, and the change is a narrowing of one expression to match three existing call sites rather than new logic. Asserting an emitted property value would require standing up component tests for two components that have none, which is a larger piece of work than this fix and better done as its own change. **Suggested follow-up, deliberately not in this PR.** `createAdvisorLintItems` and the two advisors pages filter HEALTH lints out entirely, so the category the API now returns is invisible in Studio. Whether to surface it is a product decision about a new advisor category, not a telemetry fix. Also out of scope by request: `Linter.utils.tsx` badge styling (HEALTH falling back to PERFORMANCE's badge is harmless). --- _Generated by [Claude Code](https://claude.ai/code/session_01Xwj2SotnaHByjbTfqF4Kdm)_ Co-authored-by: Claude Co-authored-by: Pamela Chia --- .../components/interfaces/ProjectHome/AdvisorSection.tsx | 6 +----- apps/studio/components/ui/AdvisorPanel/AdvisorPanel.tsx | 6 +----- packages/common/telemetry-constants.ts | 4 ++-- 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/apps/studio/components/interfaces/ProjectHome/AdvisorSection.tsx b/apps/studio/components/interfaces/ProjectHome/AdvisorSection.tsx index f26b15f611245..b0dc49349fac8 100644 --- a/apps/studio/components/interfaces/ProjectHome/AdvisorSection.tsx +++ b/apps/studio/components/interfaces/ProjectHome/AdvisorSection.tsx @@ -82,11 +82,7 @@ export const AdvisorSection = ({ showEmptyState = false }: { showEmptyState?: bo const advisorCategory = item.source === 'lint' - ? item.original.categories.includes('SECURITY') - ? 'SECURITY' - : item.original.categories.includes('PERFORMANCE') - ? 'PERFORMANCE' - : undefined + ? item.original.categories[0] : item.source === 'signal' ? 'SECURITY' : undefined diff --git a/apps/studio/components/ui/AdvisorPanel/AdvisorPanel.tsx b/apps/studio/components/ui/AdvisorPanel/AdvisorPanel.tsx index 7622d05782558..e19bd4b16740b 100644 --- a/apps/studio/components/ui/AdvisorPanel/AdvisorPanel.tsx +++ b/apps/studio/components/ui/AdvisorPanel/AdvisorPanel.tsx @@ -202,11 +202,7 @@ export const AdvisorPanel = () => { const advisorCategory = item.source === 'lint' - ? item.original.categories.includes('SECURITY') - ? 'SECURITY' - : item.original.categories.includes('PERFORMANCE') - ? 'PERFORMANCE' - : undefined + ? item.original.categories[0] : item.source === 'signal' ? 'SECURITY' : undefined diff --git a/packages/common/telemetry-constants.ts b/packages/common/telemetry-constants.ts index a164992611f51..d0e5db8d9250d 100644 --- a/packages/common/telemetry-constants.ts +++ b/packages/common/telemetry-constants.ts @@ -2959,7 +2959,7 @@ export interface AdvisorDetailOpenedEvent { */ advisorSource: 'lint' | 'notification' | 'signal' /** - * Category of the advisor (SECURITY or PERFORMANCE) + * Category of the advisor */ advisorCategory?: AdvisorCategory /** @@ -2990,7 +2990,7 @@ export interface AdvisorAssistantButtonClickedEvent { */ origin: 'homepage' | 'lint_detail' /** - * Category of the advisor (SECURITY or PERFORMANCE) + * Category of the advisor */ advisorCategory?: AdvisorCategory /** From 9d5c19e5802f5f39eba575933694baa2eb049e1a Mon Sep 17 00:00:00 2001 From: Pamela Chia Date: Mon, 31 Aug 2026 15:53:20 +0800 Subject: [PATCH 03/12] chore(repo): add public-surfaces rule to agent instructions (#49750) ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Repo maintenance: one bullet added to the committed agent instructions in `.claude/CLAUDE.md`. ## What is the new behavior? Agent sessions working in this repo (including Slack-triggered ones) get an explicit rule that PR descriptions, issues, and code comments are world-readable, so internal content stays out of them: absolute production metrics (percentages, ratios, or relative change instead), internal decision detail (vendor, legal, pricing, or strategy discussions), and competitor names (protocol identifiers such as user-agent strings are fine). That context goes in the linked Linear issue. I added this after an agent-authored PR quoted absolute internal event volumes in its description. ## Summary by CodeRabbit * **Documentation** * Added guidance to keep public PRs, issues, and code comments free of sensitive internal details. * Clarified that production metrics, internal decisions, and competitor information should be documented privately instead. --- .claude/CLAUDE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 8f48264e86e09..5ad8d8d0bf72b 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -53,6 +53,8 @@ Never hand-edit generated files: `packages/api-types/types/**`, `**/routeTree.ge **Language** — Use U.S. English everywhere. +**Public surfaces** — this repo is public: PR descriptions, issues, and code comments are world-readable. Keep internal content out of them: absolute production metrics (event counts, user counts, revenue figures: state percentages, ratios, or relative change instead), internal decision detail (vendor, legal, pricing, or strategy discussions), and competitor names (protocol identifiers such as user-agent strings are fine). Put that context in the Linear issue and link it. + ## Skills The skills in `.claude/skills/` are the source of truth for conventions — load the relevant ones before working, don't guess: From 8aade779669424cedf471985d9dd70c8d4130789 Mon Sep 17 00:00:00 2001 From: Pamela Chia Date: Mon, 31 Aug 2026 16:25:06 +0800 Subject: [PATCH 04/12] fix(www): gate changelog md alternate on slug set (#49754) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changelog entry pages advertised a `.md` alternate tag unconditionally while the page is ISR, so an entry published in the changelog repo between www deploys pointed agents at a `.md` sibling that 404s until the next build (the static file and `CHANGELOG_PAGES` are both build-time artifacts). PR #49357 made bare-URL negotiation fail closed for those entries; I gate the advertising side here the same way. **Changed:** - **No more dead `.md` links on freshly published entries**: `getStaticProps` passes a `hasMarkdownVariant` flag computed from `CHANGELOG_PAGES` membership and the page renders the alternate tag only when true. An entry published between deploys carries no tag until the build that ships its `.md` file; the set reference stays inside `getStaticProps`, so the generated module stays out of the client bundle. - **Drift coverage**: `md-alternates.test.ts` gains the changelog direction, source-level like the existing `_app.tsx` drift test; the assertion pins the full `CHANGELOG_PAGES.has(` + backtick-`changelog/${entry.slug}`-backtick + `)` expression so a dropped key prefix fails the suite, and removing the gate fails it too. **Note:** without changelog sync secrets `CHANGELOG_PAGES` is empty, so the tag never renders in local dev. Preview and prod are the verification surface. ## To test Tested on Vercel preview: - [x] Open a published changelog entry page and view source: expect `` in the head — observed exact href `/changelog/19669-supavisor-1-0.md` - [x] Fetch that href: expect 200 with `content-type: text/markdown` — observed 200, `text/markdown; charset=utf-8` - [x] (added) Client-side nav from `/changelog` into an entry: alternate tag appears with that entry's slug; hopping to a second entry updates the href (no stale tag) - [x] (added) Navigating back to `/changelog`: entry tag gone; the index shows its own pre-existing `/changelog.md` alternate (hardcoded in `pages/changelog.tsx`, outside this diff), and `/changelog.md` returns 200 `text/markdown` - [x] (added) Console: zero new errors across all scenarios vs page-load baseline ## Linear - fixes GROWTH-1120 ## Summary by CodeRabbit * **Bug Fixes** * Changelog pages now advertise a Markdown alternate link only when a Markdown version is available. * Prevented links to unavailable Markdown content from appearing on changelog entries. * **Tests** * Added coverage to verify correct Markdown alternate detection and rendering. --- apps/www/md-alternates.test.ts | 19 +++++++++++++++++++ apps/www/pages/changelog/[slug].tsx | 20 ++++++++++++++++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/apps/www/md-alternates.test.ts b/apps/www/md-alternates.test.ts index d610d6360632e..27791115c60a1 100644 --- a/apps/www/md-alternates.test.ts +++ b/apps/www/md-alternates.test.ts @@ -91,6 +91,25 @@ describe('markdown alternate drift', () => { ).toBe(true) }) + it('changelog entry page gates its .md alternate on CHANGELOG_PAGES membership', async () => { + const source = await fs.readFile( + path.join(process.cwd(), 'pages', 'changelog', '[slug].tsx'), + 'utf-8' + ) + expect( + source.includes('CHANGELOG_PAGES.has(`changelog/${entry.slug}`)'), + 'pages/changelog/[slug].tsx must compute the markdown alternate flag from CHANGELOG_PAGES membership using the changelog/-prefixed key the generator emits' + ).toBe(true) + expect( + source.includes('hasMarkdownVariant &&'), + 'pages/changelog/[slug].tsx must render the markdown alternate link only when hasMarkdownVariant is true' + ).toBe(true) + expect( + source.includes('rel="alternate" type="text/markdown"'), + 'pages/changelog/[slug].tsx must advertise the text/markdown alternate for published slugs' + ).toBe(true) + }) + it.for(MDX_SECTIONS)('%s pages advertise their .md sibling', async (urlPrefix) => { const appPagePath = path.join(process.cwd(), 'app', urlPrefix, '[slug]', 'page.tsx') if (!existsSync(appPagePath)) { diff --git a/apps/www/pages/changelog/[slug].tsx b/apps/www/pages/changelog/[slug].tsx index c27fd4281f270..692a8ab1adece 100644 --- a/apps/www/pages/changelog/[slug].tsx +++ b/apps/www/pages/changelog/[slug].tsx @@ -5,6 +5,7 @@ import { NextSeo } from 'next-seo' import Head from 'next/head' import Link from 'next/link' +import { CHANGELOG_PAGES } from '@/app/api-v2/md/content.generated' import { ChangelogDetailSidebar } from '@/components/Changelog/ChangelogDetailSidebar' import { ChangelogInlineMarkdown } from '@/components/Changelog/ChangelogInlineMarkdown' import CTABanner from '@/components/CTABanner' @@ -20,15 +21,25 @@ type PageProps = { slug: string frontmatter: ChangelogEntryFrontmatter source: MDXRemoteSerializeResult + hasMarkdownVariant: boolean } -const ChangelogDetailPage = ({ title, created_at, slug, frontmatter, source }: PageProps) => { +const ChangelogDetailPage = ({ + title, + created_at, + slug, + frontmatter, + source, + hasMarkdownVariant, +}: PageProps) => { const plainTitle = stripTitleMarkdown(title) return ( <> - - - + {hasMarkdownVariant && ( + + + + )} = async ({ params }) => { slug: entry.slug, frontmatter: entry.frontmatter, source, + hasMarkdownVariant: CHANGELOG_PAGES.has(`changelog/${entry.slug}`), }, revalidate: 900, } From 4f92790587cf39faa26ee7b5463ff18bb72477ac Mon Sep 17 00:00:00 2001 From: Gildas Garcia <1122076+djhi@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:32:45 +0200 Subject: [PATCH 05/12] fix: FormItemLayout does not apply item id correctly (#49637) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem `` does not apply item id correctly. This can be seen on https://supabase.com/design-system/docs/ui-patterns/forms: open the devtool and check the form items labels. They have no `for` attribute. This makes it harder to correctly test and is an accessibility issue. Axe devtool actually report it ## Solution When inside React Hook Form, `` actually generate an `id` (via ``). However, this `id` is overridden in `` and read from context by ``. Ensure we use the generated id unless one was provided. Also updated the paths filters for the CI check so that any changes in either `ui` or `ui-patterns` triggers the studio unit and e2e tests. ## Summary by CodeRabbit - **Bug Fixes** - Improved form accessibility by ensuring labels consistently connect to their corresponding input fields. - React-based forms now correctly preserve field-specific identifiers when associating labels with inputs. - Added support for explicitly specifying a label’s input target, improving compatibility with customized form layouts. - Updated Studio forms to use consistent control identifiers and labeling behavior. - **Quality Improvements** - Automated validation now also runs when shared UI components and patterns are updated. --- .github/workflows/studio-e2e-test.yml | 2 + .github/workflows/studio-unit-tests.yml | 4 ++ .../AccessTokens/Scoped/Form/TokenDetails.tsx | 6 +-- .../SessionsAuthSettingsForm.tsx | 12 ------ .../Vault/Secrets/EditSecretModal.tsx | 31 +++++++--------- .../Wrappers/CreateWrapperSheet.tsx | 3 +- .../Integrations/Wrappers/InputField.tsx | 7 ++-- .../Wrappers/WrapperTableEditor.tsx | 22 ++++------- .../interfaces/SQLEditor/RenameQueryModal.tsx | 2 +- .../interfaces/Storage/CreateBucketModal.tsx | 37 +++---------------- .../interfaces/Storage/EditBucketModal.tsx | 24 ++---------- .../Support/DashboardLogsToggle.tsx | 19 ++++++---- .../Support/SupportAccessToggle.tsx | 20 ++++++---- .../src/form/Layout/FormLayout.tsx | 2 +- packages/ui/src/components/shadcn/ui/form.tsx | 4 +- 15 files changed, 70 insertions(+), 125 deletions(-) diff --git a/.github/workflows/studio-e2e-test.yml b/.github/workflows/studio-e2e-test.yml index 9573189273d52..67da2f67312e2 100644 --- a/.github/workflows/studio-e2e-test.yml +++ b/.github/workflows/studio-e2e-test.yml @@ -44,6 +44,8 @@ jobs: filters: | studio: - 'packages/pg-meta/**' + - 'packages/ui/**' + - 'packages/ui-patterns/**' - 'apps/studio/**' - 'apps/ui-library/**' - 'apps/design-system/**' diff --git a/.github/workflows/studio-unit-tests.yml b/.github/workflows/studio-unit-tests.yml index bed6790239f47..65e65dabcf412 100644 --- a/.github/workflows/studio-unit-tests.yml +++ b/.github/workflows/studio-unit-tests.yml @@ -8,6 +8,8 @@ on: branches: [master, studio] paths: - 'apps/studio/**' + - 'packages/ui/**' + - 'packages/ui-patterns/**' - 'pnpm-lock.yaml' pull_request: branches: [master, studio] @@ -44,6 +46,8 @@ jobs: with: filters: | relevant: + - 'packages/ui/**' + - 'packages/ui-patterns/**' - 'apps/studio/**' - 'pnpm-lock.yaml' - uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9 diff --git a/apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/TokenDetails.tsx b/apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/TokenDetails.tsx index 64a91e603df27..a57127f6b65be 100644 --- a/apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/TokenDetails.tsx +++ b/apps/studio/components/interfaces/Account/AccessTokens/Scoped/Form/TokenDetails.tsx @@ -61,7 +61,7 @@ export const TokenDetails = ({ control, setValue }: TokenDetailsProps) => { render={({ field }) => ( - + )} @@ -72,11 +72,11 @@ export const TokenDetails = ({ control, setValue }: TokenDetailsProps) => { name="expiresAt" control={control} render={({ field }) => ( - +
+ )} @@ -162,7 +162,7 @@ export const EditSecretModal = () => { labelOptional="Optional" > - + )} @@ -173,10 +173,9 @@ export const EditSecretModal = () => { control={form.control} render={({ field }) => ( - -
+
+