fix(ui): route tag clicks to /plots and track tag_click on /stats#7516
Merged
Conversation
Tag chips on the spec/impl detail page and on the /stats tag-distribution
block built URLs as /?<param>=<value>. Since the page split, / is the
LandingPage and no longer accepts filter query params — only /plots does.
The links resolved to the landing page with stray query params and lost
the filter intent.
Fixes:
- SpecTabs.handleTagClick: navigate("/plots?…") instead of "/?…". One
call site covers both spec-overview and impl-detail since SpecPage
mounts <SpecTabs> for both router modes.
- StatsPage tag links: target /plots, encodeURIComponent the value, and
fire `tag_click` with `source: 'stats'` (StatsPage was silent on this
event — only spec_detail was tracked before).
- docs/reference/plausible.md: list StatsPage.tsx as a tag_click source
and document the `source ∈ spec_detail, stats` enum.
Drive-by: add `crypto` to the eslint browser-globals list so
FeedbackWidget.tsx (uses crypto.randomUUID / getRandomValues) stops
failing `no-undef`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes broken tag-filter links after the landing page split by routing tag clicks from spec/impl detail views and the stats tag distribution to /plots, and aligns analytics coverage by emitting tag_click from the Stats page.
Changes:
- Update SpecTabs tag-chip navigation to
/plots?...instead of/?.... - Update StatsPage tag links to
/plots?..., URL-encode tag values, and emittag_clickwithsource: 'stats'. - Update Plausible event reference docs and add
cryptoto ESLint browser globals to satisfyno-undef.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| docs/reference/plausible.md | Documents tag_click sources and includes StatsPage as an emitter. |
| app/src/pages/StatsPage.tsx | Routes tag links to /plots, URL-encodes tag values, and tracks tag_click from stats. |
| app/src/components/SpecTabs.tsx | Routes spec/impl tag-chip navigation to /plots?... while keeping analytics tracking. |
| app/eslint.config.js | Adds crypto to browser globals to avoid no-undef for Web Crypto usage. |
Comment on lines
466
to
+470
| <Link | ||
| key={tag} | ||
| component={RouterLink} | ||
| to={param ? `/?${param}=${tag}` : '/'} | ||
| to={param ? `/plots?${param}=${encodeURIComponent(tag)}` : '/plots'} | ||
| onClick={() => { if (param) trackEvent('tag_click', { param, value: tag, source: 'stats' }); }} |
Addresses Copilot review feedback on #7516: the StatsPage tag-link behavior changed in the parent commit had no test coverage (also flagged by codecov/patch/frontend). Adds one test that locks in: - the link's href is /plots?plot=scatter (regression guard for the pre-fix /?plot=scatter that silently dropped the filter intent), and - clicking fires trackEvent('tag_click', { param, value, source: 'stats' }). The useAnalytics mock is hoisted into a module-level mockTrackEvent so the assertion can see the call. Cleared per test via mockClear() because vi.restoreAllMocks() does not reset hoisted vi.fn() instances on its own. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The prior test asserted href === '/plots?plot=scatter', which proves
the /plots routing but does not exercise encodeURIComponent — 'scatter'
has no special chars, so the assertion would still pass if the encoding
were removed.
Copilot's review explicitly called out '/plots?{param}={encodedTag}',
so add a 'time series' tag (space in the value) under data_type. The
new assertion locks in href === '/plots?data=time%20series', which
fails the moment encodeURIComponent is dropped from the source. Also
clicks both links and asserts trackEvent receives the raw (unencoded)
value, matching the documented plausible.md contract.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/stats(tag distribution block) built URLs as/?<param>=<value>. Since the page split,/isLandingPageand only/plotshandles filter query params, so the links broke. Route them to/plotsinstead.tag_clickwhile SpecTabs already fired it — added the event withsource: 'stats'so Plausible coverage matches the spec.cryptowas missing from the eslint browser-globals list, makingFeedbackWidget.tsx'scrypto.randomUUID()/getRandomValues()failno-undef. Added it.Files
app/src/components/SpecTabs.tsx—handleTagClicknow navigates to/plots?…. Covers both spec-overview and impl-detail (both routes go throughSpecPage→<SpecTabs>).app/src/pages/StatsPage.tsx— link target →/plots,encodeURIComponenton the value, firetag_clickwithsource: 'stats'.docs/reference/plausible.md—tag_clicksource list now includesStatsPage.tsxand documentssource ∈ spec_detail, stats.app/eslint.config.js—crypto: 'readonly'added to browser globals.Test plan
/scatter-basic), click a tag chip under the image → URL is/plots?<param>=<value>, grid is filtered, Plausible firestag_clickwithsource: spec_detail./scatter-basic/python/matplotlib), repeat → same behaviour (same component, but verify explicitly)./stats, click a tag in the tag-distribution block → URL is/plots?<param>=<value>, grid is filtered, Plausible firestag_clickwithsource: stats.+or a space) URL-encodes correctly.yarn tsc --noEmit,yarn lint,yarn testall clean.🤖 Generated with Claude Code