feat: add core components for Mailify marketing site - #4
Merged
Conversation
- Introduced Hero component for the landing section with branding and call-to-action buttons. - Created MarketingLayout component for consistent page structure with header and footer. - Developed SiteHeader and SiteFooter components for navigation and branding. - Added ThemeToggle component for light/dark mode switching. - Implemented ThemeShowcase component to demonstrate theme system. - Created 404 error page with user-friendly messaging and navigation options. - Established pricing page detailing Mailify's cost structure and support options. - Added global and token styles for consistent theming and layout. - Configured content collections for documentation management. - Set up TypeScript configuration for improved type safety.
Reviewer's GuideAdds a new Astro-based marketing site for Mailify (home, pricing, and 404 pages) with a reusable marketing layout, core UI components, theme tokens/global styling, Starlight docs integration, and a CI workflow to build/deploy the site to GitHub Pages, plus a minor docs code-block fix. Sequence diagram for ThemeToggle behavior on the marketing sitesequenceDiagram
actor User
participant ThemeToggleButton as ThemeToggle_button
participant ThemeScript as Theme_toggle_script
participant LocalStorage
participant MediaQuery as PrefersColorScheme
participant Document as DocumentRoot
Note over ThemeScript,DocumentRoot: Initial load
ThemeScript->>LocalStorage: getItem(starlight-theme)
alt stored theme is light/dark
LocalStorage-->>ThemeScript: "light" or "dark"
else no stored theme
LocalStorage-->>ThemeScript: null
ThemeScript->>PrefersColorScheme: match('(prefers-color-scheme: light)')
PrefersColorScheme-->>ThemeScript: matches true/false
end
ThemeScript->>DocumentRoot: set data-theme to resolved theme
Note over User,ThemeToggleButton: User toggles theme
User->>ThemeToggleButton: click
ThemeToggleButton->>ThemeScript: click handler invoked
ThemeScript->>LocalStorage: getItem(starlight-theme)
ThemeScript->>PrefersColorScheme: resolveTheme(current)
PrefersColorScheme-->>ThemeScript: current effective theme
ThemeScript->>ThemeScript: compute nextTheme(light<->dark)
ThemeScript->>DocumentRoot: set data-theme = nextTheme
ThemeScript->>LocalStorage: setItem(starlight-theme, nextTheme)
Class diagram for Astro marketing layout and core componentsclassDiagram
class MarketingLayout {
+string title
+string description
+Slot content
}
class SiteHeader {
+ThemeToggle themeToggle
}
class SiteFooter {
}
class ThemeToggle {
+button[data-theme-toggle]
+getTheme()
+resolveTheme(theme)
+applyTheme(theme)
}
class Hero {
}
class FeatureGrid {
-Feature[] features
}
class Feature {
+string title
+string body
}
class CodeTabs {
-string snippet
}
class ThemeShowcase {
}
class ArchitectureDiagram {
-string[] crates
}
class IndexPage {
+MarketingLayout layout
+Hero hero
+FeatureGrid featureGrid
+CodeTabs codeTabs
+ThemeShowcase themeShowcase
+ArchitectureDiagram architectureDiagram
}
class PricingPage {
+MarketingLayout layout
}
class NotFoundPage404 {
+MarketingLayout layout
}
class AstroConfig {
+starlightIntegration
+sitemapIntegration
+customCss
+sidebarConfig
+headMetaConfig
}
class ContentCollections {
+docsCollection
}
MarketingLayout --> SiteHeader : composes
MarketingLayout --> SiteFooter : composes
SiteHeader --> ThemeToggle : uses
FeatureGrid --> Feature : aggregates
IndexPage --> MarketingLayout : uses
IndexPage --> Hero : uses
IndexPage --> FeatureGrid : uses
IndexPage --> CodeTabs : uses
IndexPage --> ThemeShowcase : uses
IndexPage --> ArchitectureDiagram : uses
PricingPage --> MarketingLayout : uses
NotFoundPage404 --> MarketingLayout : uses
AstroConfig --> ContentCollections : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
ThemeToggle.astro, consider adding a fallback tomediaQuery.addListenerwhenaddEventListener('change', ...)is not available (e.g. older Safari) to avoid runtime errors in some browsers. - Several external links (e.g. in
pricing.astroand404.astro) userel="noopener"withouttarget="_blank"; either addtarget="_blank"(andrel="noopener noreferrer") for outbound links you want to open in a new tab, or drop therelattribute if they should stay in the same tab.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ThemeToggle.astro`, consider adding a fallback to `mediaQuery.addListener` when `addEventListener('change', ...)` is not available (e.g. older Safari) to avoid runtime errors in some browsers.
- Several external links (e.g. in `pricing.astro` and `404.astro`) use `rel="noopener"` without `target="_blank"`; either add `target="_blank"` (and `rel="noopener noreferrer"`) for outbound links you want to open in a new tab, or drop the `rel` attribute if they should stay in the same tab.
## Individual Comments
### Comment 1
<location path="site/src/components/ThemeToggle.astro" line_range="30-31" />
<code_context>
+ applyTheme(nextTheme);
+ });
+
+ mediaQuery.addEventListener('change', () => {
+ if (getTheme() === 'auto') applyTheme('auto');
+ });
+</script>
</code_context>
<issue_to_address>
**issue (bug_risk):** Add a fallback for `matchMedia` change events to avoid breaking in older browsers.
Safari < 14 and some WebViews only support `mediaQuery.addListener`, so this can throw at runtime there. A small compatibility shim keeps behavior consistent:
```js
if (mediaQuery.addEventListener) {
mediaQuery.addEventListener('change', () => {
if (getTheme() === 'auto') applyTheme('auto');
});
} else if (mediaQuery.addListener) {
mediaQuery.addListener(() => {
if (getTheme() === 'auto') applyTheme('auto');
});
}
```
</issue_to_address>
### Comment 2
<location path="site/src/components/ThemeToggle.astro" line_range="1" />
<code_context>
+<button class="theme-toggle" type="button" aria-label="Toggle color theme" data-theme-toggle>
+ <span class="theme-toggle__icon theme-toggle__icon--sun" aria-hidden="true">Light</span>
+ <span class="theme-toggle__icon theme-toggle__icon--moon" aria-hidden="true">Dark</span>
</code_context>
<issue_to_address>
**suggestion:** Expose toggle state via ARIA to improve accessibility for assistive tech users.
Since this is a binary light/dark control, expose the current state via ARIA so assistive tech can announce it. For example, set `button.aria-pressed = resolveTheme(getTheme()) === 'dark'` in `applyTheme`, and update the `aria-label` or associated `aria-live` text to reflect the active theme.
Suggested implementation:
```
<button class="theme-toggle" type="button" aria-label="Activate dark color theme" aria-pressed="false" data-theme-toggle>
```
```
const getTheme = () => {
const stored = localStorage.getItem(storageKey);
return stored === 'light' || stored === 'dark' ? stored : 'auto';
};
const updateThemeToggleAria = (resolvedTheme) => {
const button = document.querySelector('[data-theme-toggle]');
if (!button) return;
const isDark = resolvedTheme === 'dark';
button.setAttribute('aria-pressed', String(isDark));
button.setAttribute(
'aria-label',
isDark ? 'Activate light color theme' : 'Activate dark color theme',
);
};
```
To fully wire this up, update the (existing) `applyTheme` logic in this file:
1. Wherever you currently resolve the effective theme (often via something like `const resolvedTheme = resolveTheme(theme)` or `resolveTheme(getTheme())`), call `updateThemeToggleAria(resolvedTheme)` after the theme has been applied to the document.
2. Ensure `applyTheme` is invoked on initial load (if it isn't already) so that `aria-pressed` and `aria-label` reflect the correct state before the user interacts with the button.
3. If the function name or signature differs (e.g. your resolver is named differently), adjust the call site accordingly, but always pass the final resolved theme string `'light'` or `'dark'` into `updateThemeToggleAria`.
</issue_to_address>
### Comment 3
<location path="site/src/components/ArchitectureDiagram.astro" line_range="24-30" />
<code_context>
+ </p>
+ </div>
+
+ <div class="diagram">
+ {crates.map((crateName, index) => (
+ <div class:list={['node', index === crates.length - 1 && 'node--primary']}>
+ {crateName}
+ </div>
</code_context>
<issue_to_address>
**suggestion:** Use list semantics for the crate diagram to improve accessibility and structure.
Since this represents a sequential flow, consider rendering it as a styled `<ol>` with `<li>` items instead of plain `<div>`s. This will preserve the current look while giving screen readers and other assistive tools a clearer sense of order and structure.
```suggestion
<ol class="diagram">
{crates.map((crateName, index) => (
<li class:list={['node', index === crates.length - 1 && 'node--primary']}>
{crateName}
</li>
))}
</ol>
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+30
to
+31
| mediaQuery.addEventListener('change', () => { | ||
| if (getTheme() === 'auto') applyTheme('auto'); |
There was a problem hiding this comment.
issue (bug_risk): Add a fallback for matchMedia change events to avoid breaking in older browsers.
Safari < 14 and some WebViews only support mediaQuery.addListener, so this can throw at runtime there. A small compatibility shim keeps behavior consistent:
if (mediaQuery.addEventListener) {
mediaQuery.addEventListener('change', () => {
if (getTheme() === 'auto') applyTheme('auto');
});
} else if (mediaQuery.addListener) {
mediaQuery.addListener(() => {
if (getTheme() === 'auto') applyTheme('auto');
});
}| @@ -0,0 +1,69 @@ | |||
| <button class="theme-toggle" type="button" aria-label="Toggle color theme" data-theme-toggle> | |||
There was a problem hiding this comment.
suggestion: Expose toggle state via ARIA to improve accessibility for assistive tech users.
Since this is a binary light/dark control, expose the current state via ARIA so assistive tech can announce it. For example, set button.aria-pressed = resolveTheme(getTheme()) === 'dark' in applyTheme, and update the aria-label or associated aria-live text to reflect the active theme.
Suggested implementation:
<button class="theme-toggle" type="button" aria-label="Activate dark color theme" aria-pressed="false" data-theme-toggle>
const getTheme = () => {
const stored = localStorage.getItem(storageKey);
return stored === 'light' || stored === 'dark' ? stored : 'auto';
};
const updateThemeToggleAria = (resolvedTheme) => {
const button = document.querySelector('[data-theme-toggle]');
if (!button) return;
const isDark = resolvedTheme === 'dark';
button.setAttribute('aria-pressed', String(isDark));
button.setAttribute(
'aria-label',
isDark ? 'Activate light color theme' : 'Activate dark color theme',
);
};
To fully wire this up, update the (existing) applyTheme logic in this file:
- Wherever you currently resolve the effective theme (often via something like
const resolvedTheme = resolveTheme(theme)orresolveTheme(getTheme())), callupdateThemeToggleAria(resolvedTheme)after the theme has been applied to the document. - Ensure
applyThemeis invoked on initial load (if it isn't already) so thataria-pressedandaria-labelreflect the correct state before the user interacts with the button. - If the function name or signature differs (e.g. your resolver is named differently), adjust the call site accordingly, but always pass the final resolved theme string
'light'or'dark'intoupdateThemeToggleAria.
Comment on lines
+24
to
+30
| <div class="diagram"> | ||
| {crates.map((crateName, index) => ( | ||
| <div class:list={['node', index === crates.length - 1 && 'node--primary']}> | ||
| {crateName} | ||
| </div> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
suggestion: Use list semantics for the crate diagram to improve accessibility and structure.
Since this represents a sequential flow, consider rendering it as a styled <ol> with <li> items instead of plain <div>s. This will preserve the current look while giving screen readers and other assistive tools a clearer sense of order and structure.
Suggested change
| <div class="diagram"> | |
| {crates.map((crateName, index) => ( | |
| <div class:list={['node', index === crates.length - 1 && 'node--primary']}> | |
| {crateName} | |
| </div> | |
| ))} | |
| </div> | |
| <ol class="diagram"> | |
| {crates.map((crateName, index) => ( | |
| <li class:list={['node', index === crates.length - 1 && 'node--primary']}> | |
| {crateName} | |
| </li> | |
| ))} | |
| </ol> |
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
1 participant
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 by Sourcery
Set up a new Astro-based marketing and documentation site for Mailify with shared marketing layout, theming, and navigation components, plus CI to build and deploy it.
New Features:
Enhancements:
Build:
CI:
Documentation: