Feature/SSD-812-ta-spinner-component#344
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
There was a problem hiding this comment.
Pull request overview
Adds a TA Spinner component preview and documentation page intended to showcase variants (color/size) for issue SSD-812.
Changes:
- Added an HTML preview page for spinner examples (with theme switching support in an iframe).
- Added spinner CSS styles (base, color variants, dark mode, size variants).
- Replaced the Spinner docs MDX page content with a tabbed preview/code layout.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| apps/docs/public/assets/ta-preview/spinner/ta-spinner-preview.html | Adds an iframe-able preview page demonstrating spinner usage in buttons. |
| apps/docs/public/assets/ta-preview/spinner/spinner.css | Introduces standalone spinner styling used by the preview and docs. |
| apps/docs/content/docs/develop/(transistory-assistance)/ta-spinner.mdx | Adds a tabbed docs layout with an iframe preview, but the embedded “Code/CSS/JS” content appears unrelated to Spinner. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -2,5 +2,573 @@ | |||
| title: Spinner | |||
| description: Explore the integrated accessibility testing tools in our development environment that help identify and resolve accessibility issues during development. These tools work together to ensure our components meet WCAG standards and provide a better experience for all users. | |||
There was a problem hiding this comment.
This Spinner docs page content looks mismatched: the frontmatter description is about accessibility testing tools, the tab labels are SelectCSS/SelectJS, and the “Code” section contains Select component markup rather than Spinner examples. Update the description and replace the Select code/CSS/JS with Spinner-specific examples and appropriately named tabs (e.g., Preview / Usage / CSS).
| import IframeThemePreview from "@/components/iframe-theme-preview"; | ||
|
|
||
| ## Work In Progress No newline at end of file | ||
| <Tabs items={["Preview","Code","SelectCSS","SelectJS"]} defaultValue="Preview"> |
There was a problem hiding this comment.
This Spinner docs page content looks mismatched: the frontmatter description is about accessibility testing tools, the tab labels are SelectCSS/SelectJS, and the “Code” section contains Select component markup rather than Spinner examples. Update the description and replace the Select code/CSS/JS with Spinner-specific examples and appropriately named tabs (e.g., Preview / Usage / CSS).
| --- | ||
| import { Tab, Tabs } from "fumadocs-ui/components/tabs"; | ||
| import IframeThemePreview from "@/components/iframe-theme-preview"; | ||
|
|
There was a problem hiding this comment.
This Spinner docs page content looks mismatched: the frontmatter description is about accessibility testing tools, the tab labels are SelectCSS/SelectJS, and the “Code” section contains Select component markup rather than Spinner examples. Update the description and replace the Select code/CSS/JS with Spinner-specific examples and appropriately named tabs (e.g., Preview / Usage / CSS).
| --- | ||
| import { Tab, Tabs } from "fumadocs-ui/components/tabs"; | ||
| import IframeThemePreview from "@/components/iframe-theme-preview"; | ||
|
|
There was a problem hiding this comment.
The fenced code blocks are labeled as tsx but contain plain HTML (including HTML comments and class=\"...\"). Use a more accurate fence language (e.g., html/css/js) so syntax highlighting and copy-paste usage are correct.
| <button type="button" class="select-trigger" label="Fruit"> | ||
| <span class="select-name">Fruit</span> | ||
| <span class="select-value">Select</span> | ||
| <svg width="2o" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg" class="text-txt-black-900 size-4" aria-hidden="true"> |
There was a problem hiding this comment.
The SVG width attribute is set to 2o (letter 'o'), which is invalid and may render incorrectly. Change it to a valid numeric value (likely 20).
| <svg width="2o" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg" class="text-txt-black-900 size-4" aria-hidden="true"> | |
| <svg width="20" height="20" viewBox="0 0 20 20" fill="none" xmlns="http://www.w3.org/2000/svg" class="text-txt-black-900 size-4" aria-hidden="true"> |
| window.addEventListener("message", (event) => { | ||
| if (event.origin !== window.origin) return; | ||
| if (event.data?.theme) { | ||
| document.documentElement.className = event.data.theme; | ||
| console.log("Theme applied inside iframe:", event.data.theme); |
There was a problem hiding this comment.
The origin check should compare against window.location.origin (not window.origin) to reliably validate the sender. Also, the console.log is noisy for docs previews; consider removing it or guarding it behind a debug flag.
| window.addEventListener("message", (event) => { | |
| if (event.origin !== window.origin) return; | |
| if (event.data?.theme) { | |
| document.documentElement.className = event.data.theme; | |
| console.log("Theme applied inside iframe:", event.data.theme); | |
| const DEBUG_THEME_MESSAGES = false; | |
| window.addEventListener("message", (event) => { | |
| if (event.origin !== window.location.origin) return; | |
| if (event.data?.theme) { | |
| document.documentElement.className = event.data.theme; | |
| if (DEBUG_THEME_MESSAGES) { | |
| console.log("Theme applied inside iframe:", event.data.theme); | |
| } |
| </head> | ||
|
|
||
| <body> | ||
| <h1>Spinner Component Examples</h1> |
There was a problem hiding this comment.
The page uses multiple <h1> elements; the second section heading should be an <h2> (or nested appropriately) for correct document structure. Also, the spinner <span> should typically be marked aria-hidden=\"true\", and if it conveys loading state you may want an accessible status (e.g., aria-busy on the button or a role=\"status\" region) to avoid screen readers reading a purely decorative element.
| @@ -0,0 +1,90 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="en" class="dark"> | |||
|
|
|||
There was a problem hiding this comment.
The page uses multiple <h1> elements; the second section heading should be an <h2> (or nested appropriately) for correct document structure. Also, the spinner <span> should typically be marked aria-hidden=\"true\", and if it conveys loading state you may want an accessible status (e.g., aria-busy on the button or a role=\"status\" region) to avoid screen readers reading a purely decorative element.
| /* Default Spinner (same as gray) */ | ||
| .spinner-default { | ||
| border-top-color: #6b6b74; | ||
| border-right-color: rgba(107, 107, 116, 0.3); | ||
| border-bottom-color: rgba(107, 107, 116, 0.3); | ||
| border-left-color: rgba(107, 107, 116, 0.3); | ||
| } |
There was a problem hiding this comment.
.spinner-default duplicates .spinner-gray exactly; consider consolidating them (e.g., shared selector .spinner-gray, .spinner-default { ... }) to avoid future drift. Also, the size variants include commented-out rem values and then hard-code px; remove the commented code and pick a single unit strategy (rem or px) to keep the stylesheet consistent.
| /* Size variants */ | ||
| .spinner-small { | ||
| /* width: 0.875rem; | ||
| height: 0.875rem; */ | ||
| width: 16px; | ||
| height: 16px; |
There was a problem hiding this comment.
.spinner-default duplicates .spinner-gray exactly; consider consolidating them (e.g., shared selector .spinner-gray, .spinner-default { ... }) to avoid future drift. Also, the size variants include commented-out rem values and then hard-code px; remove the commented code and pick a single unit strategy (rem or px) to keep the stylesheet consistent.
Summarise the feature
Issue ticket # SSD-812
Description:

TA Spinner component
Type of change
Affected endpoints
/?
Checklist