Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Pull request overview
Adds a new “Developer SDKs” marketing page and refactors shared marketing-page layout/components so the SDKs page can mirror the existing Products page structure.
Changes:
- Introduces
/sdksroute + top-nav link and a new Developer SDKs page. - Refactors the Products page to use shared
PageLayoutand sharedSection/Sectionscomponents. - Adds new SDK-related assets and styling updates (including global body reset change).
Reviewed changes
Copilot reviewed 14 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/routes/sdks.tsx | Adds the /sdks route entry under the standard layout. |
| client/src/routes/root.tsx | Updates TopNav import path to the new component location. |
| client/src/pages/sdks/DeveloperSDKs.tsx | Implements the new Developer SDKs page using PageLayout + Section(s). |
| client/src/pages/sdks/DeveloperSDKs.module.css | Adds page-specific styling (divider). |
| client/src/pages/products/Products.tsx | Refactors Products page to use shared PageLayout and shared Section(s) API. |
| client/src/pages/products/Product.module.css | Removes old page layout styles and keeps only preview styles used by the new Preview component. |
| client/src/components/top-nav/TopNav.tsx | Adds “Development SDKs” navigation link and fixes asset import path after moving the component. |
| client/src/components/top-nav/TopNav.module.css | Updates nav spacing/colors and introduces an .active class. |
| client/src/components/section/Section.tsx | Changes Section API (experience object + preview node) and adds a Sections wrapper component. |
| client/src/components/section/Section.module.css | Introduces .sections grid and removes preview styling (but leaves a mobile .preview rule). |
| client/src/components/page-layout/PageLayout.tsx | Adds shared marketing page layout component (page/content/hero). |
| client/src/components/page-layout/PageLayout.module.css | Adds shared marketing page layout styling (background, content width, hero). |
| client/src/assets/preview-js.jpg | Adds TypeScript SDK preview image asset. |
| client/src/assets/icon-unity.png | Adds Unity icon asset. |
| client/src/assets/icon-js.png | Adds JS/TS icon asset. |
| client/src/app.tsx | Registers the new sdksRoute under the standard layout. |
| client/src/app.css | Changes global body reset from all: unset to margin/padding reset. |
Comments suppressed due to low confidence (4)
client/src/components/top-nav/TopNav.tsx:33
- Naming is inconsistent across the new page: the nav label says "Development SDKs" while the page heading says "Developer SDKs", and the component is named
DevelopmentSDKsinDeveloperSDKs.tsx. Align these to a single term to avoid confusing users and to keep the API/file naming consistent.
client/src/components/section/Section.tsx:25 - The icon image alt text is hard-coded to "Swipe-able Ads", so every section will announce the wrong thing to screen readers. Use something derived from the section content (e.g.,
title) or accept aniconAltprop.
client/src/components/section/Section.tsx:29 experience.urlis collected in props but never used, and the<Button>has noonClick/navigation, so the CTA is non-functional. Either removeurlfrom the API or wire the button up to navigate/openexperience.url(e.g., render a routerLink/anchor-styled button).
client/src/components/section/Section.module.css:33- The mobile media query targets
.preview, but.previewis no longer defined in this CSS module andSectionno longer applies that class. This is dead/ineffective CSS and likely means the intended mobile spacing for previews is missing; either reintroduce a wrapper/class for previews or remove/update these rules.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- plus small UI tweaks
233f79a to
558de48
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 31 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| display: none; | ||
| } | ||
|
|
||
| .closeButton:hover { | ||
| background: var(--bolt-surface-brand-primary-hovered); | ||
| } | ||
|
|
||
| .closeButton:focus-visible { | ||
| outline: 2px solid white; | ||
| outline-offset: 2px; | ||
| } | ||
|
|
||
| .iframe { | ||
| width: 100%; | ||
| height: 100%; | ||
| border: none; | ||
| display: block; | ||
| background-color: var(--bolt-surface-primary); | ||
| } | ||
|
|
||
| @media (width >= 768px) and (height >= 500px) { | ||
| .closeButton { | ||
| display: flex; | ||
| align-items: center; | ||
| gap: var(--bolt-space-2); | ||
| } |
There was a problem hiding this comment.
On small viewports the close button is hidden (display: none by default, only shown in the >=768px media query). That leaves users without an obvious/accessible way to dismiss the <dialog> on mobile (Esc isn’t reliable there). Please ensure the modal always has a visible, keyboard-focusable close control.
| background-color: var(--bolt-surface-primary); | ||
| } | ||
|
|
||
| @media (width >= 768px) and (height >= 500px) { |
There was a problem hiding this comment.
Probably worth a comment
f9e3d85 to
43c1a1d
Compare
43c1a1d to
14ad68a
Compare
Same thing as the Products page, but for Developer SDK
All CTAs are now working