Skip to content

chore(ui): audit route heading structure - #1237

Open
QwinDivy wants to merge 2 commits into
ezedike-evan:mainfrom
QwinDivy:chore/issue-1068-heading-landmarks
Open

chore(ui): audit route heading structure#1237
QwinDivy wants to merge 2 commits into
ezedike-evan:mainfrom
QwinDivy:chore/issue-1068-heading-landmarks

Conversation

@QwinDivy

@QwinDivy QwinDivy commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

close #1068

Summary

Linked issue

Closes #

Changes

Testing notes

Automated — run the block for whatever you touched. See
CONTRIBUTING.md § Per-surface checks.

Root (always):

  • npm run format:check · ⏳ not run / ✅ green / ❌ failing
  • npm run lint · ⏳ not run / ✅ green / ❌ failing
  • npm run typecheck · ⏳ not run / ✅ green / ❌ failing
  • npm run test · ⏳ not run / ✅ green / ❌ failing
  • npm run build · ⏳ not run / ✅ green / ❌ failing
  • npm run emit-openapi → no diff, or spec committed · ⏳ / ✅ / ❌ / n-a

Rust (if contracts/ or crates/ changed):

  • cargo fmt --check · ⏳ / ✅ / ❌ / n-a
  • cargo clippy --all-targets -- -D warnings · ⏳ / ✅ / ❌ / n-a
  • cargo test --locked · ⏳ / ✅ / ❌ / n-a

Python (if packages/python-sdk/ changed):

  • pytest · ⏳ / ✅ / ❌ / n-a
  • mypy src · ⏳ / ✅ / ❌ / n-a
  • No generated file hand-edited (only wrapper.py is ours) · ⏳ / ✅ / ❌ / n-a

New / modified tests

Manual verification (if applicable)

Screenshots / recordings

Before After

Checklist

Correctness

  • The PR title follows Conventional Commits (auto-linted)
  • One logical change; unrelated cleanup was split into a separate PR
  • npm run typecheck passes
  • npm run lint passes with zero new warnings (we run --max-warnings 0 in CI)
  • npm run test passes; new behaviour has a test
  • npm run build passes

Data integrity

  • No fabricated rates, stub prices, or placeholder exchange rates (see the no-fabricated-rates rule in CONTRIBUTING.md)
  • No isMock, // MOCK, // TODO: replace with real data, or commented-out real code
  • If touching an anchor: the anchor's stellar.toml is publicly resolvable at https://{domain}/.well-known/stellar.toml and contains TRANSFER_SERVER_SEP0024
  • If touching SEP-10: network passphrase assertion is intact (mainnet only)
  • If touching SEP-24: the 10s AbortController timeout is intact on anchor fetches
  • If touching the status poll: terminal states (completed | refunded | error) still stop the SWR loop

Security & non-custody (see docs/NON_CUSTODY.md once it lands)

  • No new code path holds user keys, user funds, or long-lived anchor JWTs
  • Every signing action is performed by the user's wallet (Freighter today)
  • No secrets committed; .env.local is unchanged; new env vars are added to .env.example

Docs

  • User-facing behaviour change → CHANGELOG.md entry under [Unreleased]
  • API / schema change → relevant docs/*.md updated in the same PR
  • Architecture change → docs/ARCHITECTURE.md updated (file map, diagram, or invariants as applicable)
  • Public-facing feature → screenshot added to docs/showcase/images/ when relevant
  • New env var → .env.example + README env table updated

Release hygiene

  • If this touches a wave deliverable, the matching [ ] in docs/ROADMAP.md is updated
  • No dependency added without justification in the PR description
  • No breaking change hidden inside a non-breaking commit

Breaking changes

None.

For reviewers

@vercel

vercel Bot commented Sep 7, 2026

Copy link
Copy Markdown

@QwinDivy is attempting to deploy a commit to the ezedikeevan's projects Team on Vercel.

A member of the Team first needs to authorize it.

@ezedike-evan ezedike-evan left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking #1068 on — tests/e2e/heading-structure.spec.ts is exactly the kind of test the issue asks for, one that asserts the rule rather than the current output. That file is worth keeping.

The problem is the other half of the diff. app/anchors/page.tsx cannot be taken as written, and it is not a one-line fix — it reverts the page rather than auditing its headings:

  1. Import statements are placed inside the function body, after export default async function AnchorsPage() has already opened. That is the SyntaxError: Identifier expected. (106:7) the check (node 20) and check (node 22) jobs are failing on. Imports have to be at module scope.
  2. It redefines AnchorsContent locally. That component already exists at components/anchors/AnchorsContent.tsx and is already the client half of this page — the 'use client' boundary was deliberately moved to the component so the page could stay a React Server Component.
  3. It deletes the JSON-LD injection. The <script type="application/ld+json"> block with serializeJsonLd(jsonLd) is why this page is a server component at all; the Dataset markup has to carry a coverage window derived from real data.
  4. It reverts the design tokens. bg-blue-600, text-white and text-gray-* were replaced across the app with the theme tokens (bg-bg-subtle, text-primary-text, border-control-border, and so on). Reintroducing raw Tailwind colours breaks dark mode and the contrast test.
  5. Two default exports and an unbalanced <Suspense> — the new one opens <Suspense> and closes </>.

What I would suggest: drop app/anchors/page.tsx from this PR entirely (git checkout origin/main -- app/anchors/page.tsx) and keep only tests/e2e/heading-structure.spec.ts. Then run the spec against the pages as they are and fix the specific heading violations it catches, one page at a time — that is the audit the issue is asking for. If the spec passes everywhere already, the PR is still valuable as the regression guard #1068 wants.

Happy to look again as soon as the page file is out of the diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CHORE] [ui] Heading order and landmark audit across all fifteen routes

2 participants