fix: add focus-visible outlines for links and summary#35
fix: add focus-visible outlines for links and summary#35ronit019u wants to merge 4 commits intonitrocloudofficial:mainfrom
Conversation
cloud-nitrostack
left a comment
There was a problem hiding this comment.
Thanks for picking up accessibility and branding work — the CSS variable rename in streamable-http.ts is a solid step toward #22.
I’m requesting changes because this PR does not yet meet the acceptance criteria for the issues it links, and one change targets the wrong surface.
#24 (bundled MCP docs page — streamable-http.ts)
Issue #24 is explicitly about the server-generated documentation HTML inside generateDocumentationPage in typescript/packages/core/src/core/transports/streamable-http.ts. It asks for :focus-visible styles on a and summary (and any other interactive controls in that template), plus a quick pass on heading hierarchy.
This PR does not add any :focus-visible rules to that inline <style> block. The focus styles were added to typescript/packages/cli/templates/typescript-oauth/src/widgets/app/globals.css instead. Per #24, scaffolded template apps are out of scope for closing that issue — so the oauth change is a nice extra for that template, but it does not satisfy #24.
What would complete #24: add focused, specific selectors in the streamable-http.ts template CSS (e.g. .footer a:focus-visible, .tool-schema summary:focus-visible, etc.) with a visible outline and offset, then sanity-check tab order in the served docs page.
#22 (branding — partial)
Renaming --nitrocloud-* → --nitrostack-* matches part of #22. The bundled page still uses alt="NitroCloud Logo" on the logo <img> in the same template; #22 also calls out updating that alt to NitroStack (and optionally aligning the primary hex with branding.ts). Please include the alt text (and any remaining “NitroCloud” strings in that HTML/CSS block) in this PR or split #22 into a follow-up with a clear scope note.
Small hygiene
globals.cssends without a trailing newline (\ No newline at end of filein the patch). Worth fixing for consistency.
PR description / checklist
The template still has empty “Changes Made” bullets and unchecked testing items. Once the streamable-http focus + branding pieces are in place, a short note on how you verified the docs page (browser tab through footer link + tool schema <summary>) would help reviewers a lot.
Happy to re-review after the streamable-http.ts template updates land — that’s the critical path for closing #24.
cloud-nitrostack
left a comment
There was a problem hiding this comment.
Thanks for taking on accessibility for the bundled docs — renaming the CSS variables to nitrostack-* is a nice consistency win, and adding :focus-visible for links/summary is directionally exactly what we need.
A few friendly requests before we merge:
-
Focus styles scope: In
streamable-http.ts, the newa:focus-visible/summary:focus-visiblerules appear inside the@media (prefers-color-scheme: dark)block. That means users in light mode won’t get those outlines. Could you move the focus-visible rules to the global<style>section (so they apply in both themes), and keep theme-specific tweaks only where necessary? -
Issue #24 heading semantics: The issue also asks for sensible heading semantics in
generateDocumentationPage(not only CSS). If you have bandwidth, a small pass on the generated markup (e.g. hierarchy / landmark-friendly structure) would help us fully close #24. If you prefer to keep this PR CSS-only, we can drop “Closes #24” and treat the oauthglobals.csschange as a separate nice-to-have — totally your call. -
PR hygiene: Opening from your fork’s
mainbranch can make updates harder for you long-term; a short-lived feature branch next time would be easier to maintain. -
Checklist: When you get a chance, running through lint / tests and filling in the PR template (screenshots of focus rings in light + dark) would make review quicker.
We’re grateful you’re here — NitroStack is open source because of contributors like you. If the project’s useful, a ⭐ on github.com/nitrocloudofficial/nitrostack helps others discover it, and we’d love more PRs (or referrals to dev friends) whenever you’re up for it.
Description
It adds focus-visible outlines for links and summary
Type of Change
Related Issues
Closes #24
Changes Made
Testing
npm run lintnpm testScreenshots / Recordings
Checklist
CONTRIBUTING.md