Theme config, launcher integration, dependency updates and several QoL enhancements#171
Conversation
…ing local development
…unctions - Introduced `get-breakpoint-value` function for improved breakpoint value retrieval. - Enhanced `breakpoint` mixin to support min, max, and range queries. - Added utility mixins: `breakpoint-max`, `breakpoint-min`, and `breakpoint-between` for clarity and ease of use.
…ructure - Integrated PageHeader to display dynamic titles and subtitles based on frontmatter. - Adjusted container class based on frontmatter property for responsive design.
… for improved content management - Deleted the old Astro component for the accessibility statement. - Added a new markdown file to streamline content updates and maintain accessibility standards.
…onfiguration - Added support for loading the blog API URL from environment variables. - Updated Astro configuration to utilize the SITE_URL from the environment. - Refactored blog page components to fetch posts from the configured API URL. - Added .env.example - Updated typescript notations
- Added theme configuration file to define theme properties such as name, logo, and SEO details. - Updated Logo component to utilize the new theme configuration for logo and name display. - Enhanced SiteMeta component to pull default values from theme configuration for title, description, and author. - Refactored various imports to use the new theme configuration utility. - Introduced utility function to define and export theme configuration.
- Introduced a new NavigationItems component to dynamically render navigation links based on the theme configuration. - Updated theme configuration to include navigation settings, such as dark mode support and structured navigation items. - Refactored Navigation component to utilize the new NavigationItems for rendering menu items.
…items - Replaced the hardcoded navigation items in the Header component with a single Navigation component for improved maintainability and flexibility.
… for improved features and fixes
…mproved design consistency
…current situation
Integrated the accessible-astro-launcher components into the header, providing a launcher trigger for desktop navigation and quick access to site pages and accessibility preferences (dark mode, high contrast, reduced motion). Updated menu structure and styles to accommodate the new launcher and ensure accessibility features are initialized.
Integrated the LauncherTrigger component from 'accessible-astro-launcher' into the navigation for mobile devices. Updated styles to display the launcher only on mobile and hide it on desktop, improving accessibility and user experience.
Introduced HighContrast and ReducedMotion components to the accessible components page, including default and custom icon examples. Updated the components list and navigation to include these new toggles.
Introduced a new path alias '@theme-config' in the Vite configuration to simplify imports from 'theme.config.ts'.
Deleted src/utils/themeConfig.ts and added a new @theme-config path alias in tsconfig.json to reference theme.config.ts directly. This simplifies theme configuration imports.
Expanded and refined the NavigationItem type to support dropdowns and additional properties. Added SocialItem type and socials array to the theme config. Updated theme.config.ts to use the new navigation and socials structure, and included the new 'outline' color. Improved default handling in defineThemeConfig.
Changed the import path for themeConfig from '@utils/themeConfig' to '@theme-config' to reflect the updated module location or alias.
Updated the import path for themeConfig and refactored the way theme color variables are injected into the stylesheet. Now uses define:vars and :global(:root) for CSS variables, improving maintainability and clarity.
Included the 'accessible-astro-launcher' package version 1.0.0 in the project dependencies to support new functionality or features requiring this package.
Updated the package version from 4.3.2 to 5.0.0 to reflect new changes or a major release.
Replace the custom SiteMeta component with the astro-seo package for better SEO management. This provides more robust and standardized SEO handling with improved Open Graph and Twitter Card support. - Add astro-seo package dependency - Remove custom SiteMeta.astro component - Integrate SEO component directly in DefaultLayout with enhanced configuration - Update image type to support both ImageMetadata and string paths - Pass SEO props through MarkdownLayout to DefaultLayout - Update homepage with custom title and disable title template BREAKING CHANGE: SiteMeta component has been removed. Pages using DefaultLayout should now pass SEO props directly to the layout instead of using SiteMeta.
Removed the social preview image from the public directory and updated the image in src/assets/img. This change may be part of an asset reorganization or image update for social sharing.
Introduces a new page showcasing the Accessible Astro Launcher component, including usage instructions and links to documentation for accessible development.
Introduces a new navigation item labeled 'Accessible launcher' with a link to '/accessible-launcher' in the theme configuration.
Bump various devDependencies and dependencies to their latest versions for improved features, bug fixes, and compatibility.
Applied consistent code formatting across Astro components, utility files, and configuration files. Standardized quote usage in project content files and YAML, removed unnecessary Prettier config options, and improved code readability by collapsing multi-line elements and objects. No functional changes were made.
Removed the launcher functionality from the Header component and created a new LauncherConfig component to handle accessibility preferences, navigation, blog posts, projects, and social links. This change improves code organization and separates concerns for better maintainability.
✅ Deploy Preview for accessible-astro-starter-incluud ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds a theming/config system, a data-driven command launcher with accessibility preferences, refactors header/navigation to integrate the launcher and accessibility toggles, adds a workspace Vite helper for linked packages, enables env-driven blog fetching, enhances SCSS utilities/tokens, replaces SiteMeta with astro-seo, and bumps dependencies to 5.0.0. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client as Launcher UI
participant Server as Astro Server
participant API as BLOG_API_URL
User->>Client: open launcher (Cmd/Ctrl+K) / type
Client->>Server: request launcher data (themeConfig, projects, nav, prefs)
alt BLOG_API_URL set
Server->>API: fetch posts from BLOG_API_URL
API-->>Server: return posts JSON
else no BLOG_API_URL
Server-->>Server: skip fetch (return empty posts)
end
Server-->>Client: return launcher items (preferences, posts, projects, socials)
Client->>User: render launcher list and handle actions/links
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (17 files)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/portfolio/[...page].astro (1)
49-49: Remove debugconsole.logstatement.This appears to be leftover debug code that should be removed before merging. It will pollute server logs in production.
🧹 Proposed fix
const { page, uniqueTags } = Astro.props - -console.log(page.lastPage) ---
🤖 Fix all issues with AI agents
In @.prettierrc:
- Line 6: The plugins array currently lists "prettier-plugin-tailwindcss" before
"prettier-plugin-css-order", which causes formatting conflicts; update the
plugins array so "prettier-plugin-tailwindcss" appears last by reordering it to
["prettier-plugin-astro", "prettier-plugin-css-order",
"prettier-plugin-tailwindcss"] (adjust the "plugins" entry accordingly) to
ensure tailwind's plugin runs after css-order.
In `@astro.config.mjs`:
- Line 8: Remove the unused import by deleting the import specifier "loadEnv"
from the top of the config (the import statement that reads import { loadEnv }
from 'vite'); ensure no other code references the symbol "loadEnv" in this file
and update the import line accordingly so the file no longer imports an unused
symbol.
In `@README.md`:
- Around line 41-43: The README feature list incorrectly references non-existent
components HighContrast.astro and ReducedMotion.astro; update the text to
reference the actual component ColorContrast.astro (and any other real
preference/contrast components exposed by the codebase) and remove or rename the
invalid entries; verify the exported component names (e.g., ColorContrast.astro
and any motion-preference component) and update the line that lists these
features so it only mentions real components and keeps the existing command
launcher note unchanged.
In `@scripts/workspace-config.js`:
- Around line 62-79: The code currently overwrites Vite config keys; instead
merge safely: ensure baseConfig.resolve exists before setting preserveSymlinks
(e.g., baseConfig.resolve = baseConfig.resolve ?? {}, then set
baseConfig.resolve.preserveSymlinks = true), merge server.fs.allow with any
existing allow array rather than replacing it (baseConfig.server = {
...(baseConfig.server ?? {}), fs: { allow: Array.from(new
Set([...(baseConfig.server?.fs?.allow ?? []), '..', '../..'])) } }), merge
optimizeDeps.exclude by combining linkedPackages with existing exclude (use Set
to dedupe) and merge ssr.noExternal similarly, and append the plugin to
baseConfig.plugins by initializing baseConfig.plugins = baseConfig.plugins ?? []
and pushing the new plugin rather than assuming it exists.
In `@src/components/LauncherConfig.astro`:
- Around line 53-69: The fetch block calling BLOG_API_URL lacks a response.ok
check before calling response.json(), so add an explicit check of response.ok
after the fetch (before parsing) and handle non-2xx responses by
logging/throwing or returning an empty array; specifically modify the async
block that fetches BLOG_API_URL (used with truncateBlogTitle and
maxLauncherBlogPosts) to await fetch, check response.ok, and only call
response.json() when ok is true, otherwise return [] (or throw to hit the catch)
so downstream code that maps to { title: string } is never given an unexpected
response body.
- Around line 58-65: The shortUrl generation using truncatedTitle.replaceAll('
', '-').toLowerCase() can produce invalid slugs for special characters; update
the mapping in the return of the data.slice(...).map callback (where
truncatedTitle is created by truncateBlogTitle) to sanitize the slug: either
import a slugify library (e.g., slugify) and call it on truncatedTitle, or
implement normalization by running truncatedTitle.normalize('NFKD') to strip
accents, remove all non-alphanumeric/space characters, replace spaces with
hyphens, collapse multiple hyphens into one, trim leading/trailing hyphens, and
then toLowerCase(), and use that result for href (`/blog/${sanitizedSlug}`).
In `@src/components/Navigation.astro`:
- Around line 22-24: The icon-only LauncherTrigger lacks an accessible name;
update the instance of LauncherTrigger (the element with
launcherId="site-launcher" and iconOnly={true}) to supply an accessible label by
adding the placeholder prop (e.g., placeholder="Open launcher") or the
appropriate aria-label prop if accessible-astro-launcher uses a different prop —
mirror how NavigationItems.astro provides placeholder, or consult the
accessible-astro-launcher docs and pass that prop to LauncherTrigger so the
icon-only button has an accessible name.
In `@src/components/NavigationItems.astro`:
- Around line 25-36: The render condition treats item.type === 'link' strictly
so items with no type (undefined) don't render; update the check in
NavigationItems.astro to treat undefined as 'link' (e.g., use (item.type ??
'link') === 'link' or !item.type || item.type === 'link') so the Link branch
(component Link, Icon usage and sr-only span) renders for items missing an
explicit type while preserving other branches like groups.
In `@src/layouts/DefaultLayout.astro`:
- Line 26: The type ImageMetadata is referenced in DefaultLayout.astro but not
imported; add an import for ImageMetadata from the 'astro' package at the top of
the file (e.g., import type { ImageMetadata } from 'astro') so the declaration
image?: ImageMetadata | string | null resolves and the TypeScript/ESLint error
is eliminated.
- Around line 43-47: The code mixes optional chaining for themeConfig.seo
properties: themeConfig.seo.title vs themeConfig.seo?.description/image/author,
which can throw if seo is undefined; make access consistent by either ensuring
seo is always present or guarding the title access. Update the assignment for
title to use the same safe access pattern (e.g., themeConfig.seo?.title ?? '' or
provide a default seo object before these lines) so title uses the same optional
chaining/Defaults as description/image/author and avoid runtime errors in
DefaultLayout.astro.
In `@src/pages/blog/`[post].astro:
- Around line 20-25: The getStaticPaths function currently does an unguarded
fetch(BLOG_API_URL) which can throw or return non-OK responses and crash static
generation; wrap the fetch in a try-catch inside getStaticPaths, await
fetch(BLOG_API_URL), check response.ok before calling response.json(), and
return [] on any fetch error or non-ok response (mirror the existing behavior
when BLOG_API_URL is missing); apply the same pattern to the getStaticPaths
implementation in the similar [...page].astro file.
In `@src/pages/index.astro`:
- Around line 156-158: The features marketing copy referencing "25+ components"
is now out of sync with the Counter components (e.g., <Counter count="35+"
title="Accessible" sub="Components" />); update that textual copy to "35+
components" so it matches the Counter, locating the string in the features
section of src/pages/index.astro (look for the human-readable phrase "25+
components" near the Counter usage) and replace it with "35+ components".
🧹 Nitpick comments (14)
src/assets/scss/base/_root.scss (1)
22-23: Outline palette lightness scale is inverted compared to other palettes.The other color palettes use decreasing lightness as numbers increase (e.g.,
primary-100at 90%,primary-200at 80%). Here,outline-100is 50% (darker) andoutline-200is 80% (lighter), which reverses the expected pattern.Consider aligning with the existing convention for consistency:
♻️ Suggested alignment with existing palette convention
- --color-outline-100: oklch(from var(--brand-outline) 50% c h); - --color-outline-200: oklch(from var(--brand-outline) 80% c h); + --color-outline-100: oklch(from var(--brand-outline) 80% c h); + --color-outline-200: oklch(from var(--brand-outline) 50% c h);Then update the light-dark usage on line 46 accordingly:
- --outline-color: light-dark(var(--color-outline-100), var(--color-outline-200)); + --outline-color: light-dark(var(--color-outline-200), var(--color-outline-100));src/assets/scss/base/_breakpoint.scss (1)
18-18: Prefertype-ofovertype_offor consistency.While Sass treats hyphens and underscores as interchangeable, the convention is to use hyphens to align with CSS naming conventions.
✏️ Suggested fix
- } `@else` if (type_of($value) == number) { + } `@else` if (type-of($value) == number) {.env.example (1)
1-2: Minor: Add trailing newline and use consistent quote style.The file is missing a trailing newline and uses inconsistent quote styles (double quotes on line 1, single quotes on line 2). Consider standardizing for consistency.
Suggested fix
-SITE_URL="https://accessible-astro-starter.incluud.dev" -BLOG_API_URL='https://jsonplaceholder.typicode.com/posts' +SITE_URL="https://accessible-astro-starter.incluud.dev" +BLOG_API_URL="https://jsonplaceholder.typicode.com/posts" +src/pages/accessible-components.astro (2)
386-394: Consider using more semantically relevant icons for the demo.The
lucide:sunriseandlucide:sunseticons work as a generic toggle demo, but for a high contrast toggle, icons likelucide:contrastorlucide:eye/lucide:eye-offmight better communicate the feature's purpose to users browsing this components showcase.
510-518: Consider using motion-related icons for clarity.Similar to the HighContrast section, the sunrise/sunset icons don't semantically convey "reduced motion." Icons like
lucide:pause/lucide:playorlucide:activitymight better illustrate the toggle's purpose in this demo context.src/utils/defineThemeConfig.ts (1)
3-12: Remove redundant type intersection.Line 12 has a duplicate
& { type?: 'link' }that's already declared on line 5. This intersection is redundant and adds confusion.♻️ Proposed fix
export type NavigationItem = | ({ type?: 'link' label: string href: string external?: boolean highlight?: boolean icon?: string excludeFromLauncher?: boolean - } & { type?: 'link' }) // This allows 'type' to be optional, defaulting to 'link' by intent. + }) | { type: 'dropdown'The comment's intent (making
typeoptional, defaulting to 'link') is already achieved bytype?: 'link'on line 5.src/components/NavigationItems.astro (1)
37-49: Remove redundant type check.The condition on line 39 (
item.type === 'dropdown') is redundant since this block is already inside a{item.type === 'dropdown' && ...}guard on line 37.♻️ Proposed fix
{item.type === 'dropdown' && ( <ul class="dropdown-menu"> - {item.type === 'dropdown' - ? item.items.map((subItem) => ( + {item.items.map((subItem) => ( <li class="submenu-item"> <Link href={subItem.href} isExternal={subItem.external ?? false}> {subItem.label} </Link> </li> - )) - : null} + ))} </ul> )}src/pages/accessible-launcher.astro (1)
2-3: Use path aliases for layout/component imports.♻️ Suggested change
-import DefaultLayout from '../layouts/DefaultLayout.astro' -import PageHeader from '../components/PageHeader.astro' +import DefaultLayout from '@layouts/DefaultLayout.astro' +import PageHeader from '@components/PageHeader.astro'As per coding guidelines, Use path aliases for imports:
@components,@layouts,@assets,@content,@pages,@public,@post-images,@project-images.src/components/Logo.astro (1)
23-28: Avoid redundant logo announcements for screen readers.Since the site name is already rendered as text, the logo image can be decorative to prevent double announcements.
♻️ Suggested change
- <Image src={themeConfig.logo} alt={`${themeConfig.name} logo`} width="47" height="37" loading={loading} /> + <Image + src={themeConfig.logo} + alt="" + aria-hidden="true" + width="47" + height="37" + loading={loading} + />As per coding guidelines, Provide meaningful alt text for images or use alt="" for decorative images.
src/pages/blog/[...page].astro (1)
6-30: Add explicit fetch error handling for resilience and observability.The
getStaticPathsfunction silently returns an empty array whenBLOG_API_URLis missing, and does not validate the fetch response before calling.json(). This can hide network failures or API errors. Consider addingresponse.okchecking and logging warnings.The codebase uses try/catch patterns for similar fetch operations (e.g., in
LauncherConfig.astro), so you could either adopt that approach or use explicit response validation as suggested:- if (!BLOG_API_URL) { - return [] - } - const data = await fetch(BLOG_API_URL).then((response) => response.json()) + if (!BLOG_API_URL) { + console.warn('BLOG_API_URL not set; skipping blog generation.') + return [] + } + + const response = await fetch(BLOG_API_URL) + if (!response.ok) { + console.warn(`BLOG_API_URL request failed (${response.status}); skipping blog generation.`) + return [] + } + const data = await response.json()scripts/workspace-config.js (1)
83-105: Replace fs.watch with Vite's server.watcher for cross-platform reliability.
fs.watch()withrecursive: trueonly works reliably on macOS and Windows; Linux's inotify backend does not support recursive watching. Vite'sserver.watcher(Chokidar) provides cross-platform recursive watching and integrates directly with the dev server lifecycle.Consider refactoring to use
server.watcher.add()andserver.watcher.on('change'). Note that listeners registered onserver.watchershould typically be set up once during plugin initialization, as multiple registrations could cause duplicate event handlers. Vite manages the watcher cleanup when the server closes.src/pages/blog/[post].astro (1)
10-17: Replaceanywith a concrete post type for strict mode.
featuredImage: anyandpost: anyerode type safety. Consider defining an API post type and usingfeaturedImage: string(orImageMetadataif you switch to asset imports).♻️ Suggested typing cleanup
-interface Props { - post: { - id: number - userId: number - title: string - body: string - featuredImage: any - } -} +interface ApiPost { + id: number + userId: number + title: string + body: string +} + +interface Props { + post: ApiPost & { + featuredImage: string + } +} @@ - return data.map((post: any) => { + return data.map((post: ApiPost) => {Also applies to: 44-45
src/components/Header.astro (1)
2-4: Consider using path aliases for component imports.The imports use relative paths instead of the
@componentsalias defined in the project. While functional, using aliases improves consistency and makes refactoring easier.♻️ Suggested refactor
--- -import Navigation from '../components/Navigation.astro' -import LauncherConfig from './LauncherConfig.astro' +import Navigation from '@components/Navigation.astro' +import LauncherConfig from '@components/LauncherConfig.astro' import { HighContrast, ReducedMotion, SkipLink } from 'accessible-astro-components'As per coding guidelines: "Use path aliases for imports:
@components,@layouts,@assets..."src/components/LauncherConfig.astro (1)
120-125: Type assertion suggests incomplete type definition.The cast
(item as { external?: boolean }).externalindicates thesocialstype inthemeConfigmay not include theexternalproperty. Consider updating the type definition in the theme config to include this property, which would provide better type safety.
| title = themeConfig.seo.title, | ||
| description = themeConfig.seo?.description || '', | ||
| url = Astro.url.href, | ||
| image = themeConfig.seo?.image || null, | ||
| author = themeConfig.seo?.author || '', |
There was a problem hiding this comment.
Inconsistent optional chaining on themeConfig.seo properties.
Line 43 accesses themeConfig.seo.title without optional chaining, while lines 44, 46, and 47 use themeConfig.seo?.description, themeConfig.seo?.image, and themeConfig.seo?.author. If seo can be undefined, line 43 will throw; if it's always defined, the ?. operators are unnecessary.
🔧 Suggested fix (assuming `seo` is always defined)
const {
title = themeConfig.seo.title,
- description = themeConfig.seo?.description || '',
+ description = themeConfig.seo.description || '',
url = Astro.url.href,
- image = themeConfig.seo?.image || null,
- author = themeConfig.seo?.author || '',
+ image = themeConfig.seo.image || null,
+ author = themeConfig.seo.author || '',
type = 'website',
useTitleTemplate = true,
} = Astro.props📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| title = themeConfig.seo.title, | |
| description = themeConfig.seo?.description || '', | |
| url = Astro.url.href, | |
| image = themeConfig.seo?.image || null, | |
| author = themeConfig.seo?.author || '', | |
| title = themeConfig.seo.title, | |
| description = themeConfig.seo.description || '', | |
| url = Astro.url.href, | |
| image = themeConfig.seo.image || null, | |
| author = themeConfig.seo.author || '', |
🤖 Prompt for AI Agents
In `@src/layouts/DefaultLayout.astro` around lines 43 - 47, The code mixes
optional chaining for themeConfig.seo properties: themeConfig.seo.title vs
themeConfig.seo?.description/image/author, which can throw if seo is undefined;
make access consistent by either ensuring seo is always present or guarding the
title access. Update the assignment for title to use the same safe access
pattern (e.g., themeConfig.seo?.title ?? '' or provide a default seo object
before these lines) so title uses the same optional chaining/Defaults as
description/image/author and avoid runtime errors in DefaultLayout.astro.
markteekman
changed the title
…minifier doesn't recognize the syntax
Introduced a new section in the LauncherConfig component for accessible Astro projects, including links to various resources such as the Accessible Astro Starter, Dashboard, Components, Docs, and Color Contrast Checker. Enhanced the LauncherItem components with icons for improved visual representation.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/Header.astro`:
- Around line 14-21: Move the <SkipLink /> component so it appears before the
focusable preference toggles; specifically, reorder the JSX in Header.astro so
SkipLink is placed above HighContrast and ReducedMotion (which render
interactive buttons) to ensure SkipLink is the first focusable element; keep the
existing <Navigation />, <LauncherConfig /> placements unchanged.
In `@src/components/LauncherConfig.astro`:
- Around line 107-118: The launcher mapping currently only includes items where
item.type === 'link', so items with type undefined are excluded; update the
condition in the mapping inside LauncherConfig.astro to treat undefined as a
link (e.g., change the check to accept item.type === 'link' OR item.type ===
undefined / falsy) so the branch that returns the link object (label, href,
external) also runs for items missing an explicit type (reference the item
variable and the mapping that returns the { label, href, external } object).
In `@src/pages/blog/`[post].astro:
- Line 9: The Vite runtime is missing the '@utils' alias used by imports like
slugify from '@utils/slugify'; update the Vite config's resolve.alias (in the
viteConfig object) to map '@utils' to the src/utils directory using
fileURLToPath(new URL('./src/utils', import.meta.url)) so runtime imports match
the tsconfig path mapping; ensure fileURLToPath and URL are imported/available
in the config file if needed.
🧹 Nitpick comments (5)
src/components/LauncherConfig.astro (1)
203-213: Inconsistent external link handling for Accessible Astro Projects.The
launcherAccessibleAstroProjectsitems are external GitHub links but don't includetarget="_blank"andrel="noopener noreferrer", unlikelauncherNavigationItems(line 176) andlauncherSocialItems(line 222). Consider adding anexternalflag for consistency.♻️ Proposed fix
Update the data structure:
const launcherAccessibleAstroProjects = [ { label: 'Accessible Astro Starter', href: 'https://github.com/incluud/accessible-astro-starter', + external: true, }, // ... similar for other items ]And update the render:
{launcherAccessibleAstroProjects.map((item) => ( - <LauncherItem type="navigation" label={item.label} href={item.href}> + <LauncherItem + type="navigation" + label={item.label} + href={item.href} + {...(item.external ? { target: '_blank', rel: 'noopener noreferrer' } : {})} + > <Icon slot="icon" aria-hidden="true" name="lucide:external-link" size="16" /> </LauncherItem> ))}src/pages/blog/[post].astro (1)
76-78: Consider simplifying the canonical URL fallback.The fallback
Astro.site ?? Astro.urlis reasonable, but ifAstro.siteis not configured, the canonical URL will use the request URL which may not be the intended production URL.💡 Optional: Add a warning when Astro.site is undefined
const { post } = Astro.props as Props const postSlug = Astro.params.post ?? slugify(post.title) +if (!Astro.site) { + console.warn('Astro.site is not configured; canonical URL may be incorrect.') +} const canonicalUrl = new URL(`/blog/${postSlug}`, Astro.site ?? Astro.url).hrefsrc/pages/blog/[...page].astro (1)
56-56: Document the post limit for clarity.The
data.slice(0, 30)silently limits posts to 30. Consider adding a comment or extracting this as a constant to make the intent explicit.💡 Add a clarifying comment
+ // Limit to first 30 posts for demo purposes const postsWithImages = data.slice(0, 30).map((post) => {scripts/workspace-config.js (1)
99-100: Hardcoded sibling directory paths may be fragile.The paths assume specific sibling directory structure (
../accessible-astro-components/src/components). Consider making these paths configurable or documenting the expected workspace layout.💡 Consider adding path configuration or clearer documentation
+ // Note: These paths assume packages are in sibling directories at the same level + // as this repository (e.g., ../accessible-astro-components/) const componentsPath = resolve('../accessible-astro-components/src/components') const launcherPath = resolve('../accessible-astro-launcher/src')src/components/Header.astro (1)
24-26: Prefer@assetsalias for the SCSS import.This keeps import paths consistent with project conventions.
♻️ Proposed change
- `@use` '../assets/scss/base/breakpoint' as *; + `@use` '@assets/scss/base/breakpoint' as *;As per coding guidelines: Use path aliases for imports:
@components,@layouts,@assets,@content,@pages,@public,@post-images,@project-images.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/Header.astro`:
- Around line 15-17: HighContrast and ReducedMotion are rendered with "sr-only"
which hides them visually but leaves them focusable and thus creates invisible
tab stops; fix by removing them from the tab order when used only for
initialization — add tabindex="-1" and aria-hidden="true" to the HighContrast
and ReducedMotion elements in the Header component (or move their rendered UI to
a visible, accessible location elsewhere) so keyboard users are not trapped by
invisible focusable controls.
🧹 Nitpick comments (1)
scripts/workspace-config.js (1)
1-2: Derive watch paths from the symlink target to avoid missed reloads.Line 97-105 assumes sibling directories; if the symlink points elsewhere, changes won’t trigger reloads. Resolve the real path from
node_moduleswhen symlinks are detected.♻️ Suggested update
-import { existsSync, lstatSync } from 'fs' +import { existsSync, lstatSync, realpathSync } from 'fs' @@ - const componentsPath = resolve('../accessible-astro-components/src/components') - const launcherPath = resolve('../accessible-astro-launcher/src') + const componentsPackage = resolve('./node_modules/accessible-astro-components') + const launcherPackage = resolve('./node_modules/accessible-astro-launcher') + const componentsPath = isComponentsLinked + ? resolve(realpathSync(componentsPackage), 'src/components') + : null + const launcherPath = isLauncherLinked ? resolve(realpathSync(launcherPackage), 'src') : null @@ - if (isComponentsLinked) { + if (componentsPath) { watchPaths.push(componentsPath) } - if (isLauncherLinked) { + if (launcherPath) { watchPaths.push(launcherPath) }Also applies to: 95-106
1 participant
Summary by CodeRabbit
New Features
Components
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.