Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces the Embla router to enable single-page application functionality with dynamic routing and page components. The implementation replaces the static content with a router-based navigation system and includes GitHub Pages compatibility through a custom redirect mechanism.
Changes:
- Integrated the Embla router library and created five page components (home, about, articles, article, and 404)
- Replaced static HTML content with router configuration supporting both static and dynamic routes
- Added GitHub Pages SPA support through a redirect script in 404.html and query parameter handling in index.html
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.ts | Added router and page component imports to initialize the SPA |
| src/components/page-home/page-home.tsx | Created home page component with navigation links |
| src/components/page-about/page-about.tsx | Created about page component with router demonstration description |
| src/components/page-articles/page-articles.tsx | Created articles listing page with sample article links |
| src/components/page-article/page-article.tsx | Created dynamic article page that displays route parameters |
| src/components/page-notfound/page-notfound.tsx | Created 404 error page component |
| index.html | Replaced static content with router configuration and added SPA redirect script |
| 404.html | Created GitHub Pages redirect handler to support client-side routing |
| this.render(); | ||
| } | ||
|
|
||
| attributeChangedCallback(_name: string, oldValue: string, newValue: string) { |
There was a problem hiding this comment.
The parameter '_name' is prefixed with underscore to indicate it's unused, but it's included in the observedAttributes list. Consider renaming to 'name' if it's meaningful, or document why it's intentionally unused.
| attributeChangedCallback(_name: string, oldValue: string, newValue: string) { | |
| attributeChangedCallback(name: string, oldValue: string, newValue: string) { |
| const pathSegments = window.location.pathname.slice(1).split('/').filter(Boolean); | ||
| const redirectPath = pathSegments.length > 0 | ||
| ? '/?p=/' + pathSegments.join('/') | ||
| : '/'; |
There was a problem hiding this comment.
The redirect logic constructs the path with string concatenation which could lead to double slashes. When pathSegments is empty but the original path had a trailing slash, or when joining segments that might contain edge cases. Consider using a more robust path construction approach or add a comment explaining the expected input format.
| const pathSegments = window.location.pathname.slice(1).split('/').filter(Boolean); | |
| const redirectPath = pathSegments.length > 0 | |
| ? '/?p=/' + pathSegments.join('/') | |
| : '/'; | |
| const pathSegments = window.location.pathname | |
| .slice(1) | |
| .split('/') | |
| .filter(Boolean); | |
| function buildRedirectPath(segments) { | |
| if (!segments || segments.length === 0) { | |
| return '/'; | |
| } | |
| // Normalize segments to avoid accidental double slashes | |
| const normalized = segments | |
| .map(function (segment) { | |
| return segment.replace(/^\/+|\/+$/g, ''); | |
| }) | |
| .filter(Boolean) | |
| .join('/'); | |
| return normalized ? '/?p=/' + normalized : '/'; | |
| } | |
| const redirectPath = buildRedirectPath(pathSegments); |
1 participant
Introduce the Embla router to enable single-page application functionality, allowing for dynamic routing and page components. This update includes the creation of various page components and a redirect script for GitHub Pages.