Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion post.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ if (fs.existsSync(path.join(distDir, 'index.html'))) {
const targetDir = path.join(distDir, post.year, post.month, post.day);

if (!fs.existsSync(targetDir)) {
fs.mkdirSync(targetDir, { recursive: true });
fs.mkdirSync(targetDir, {
recursive: true
});
}

fs.copyFileSync(
Expand All @@ -68,6 +70,11 @@ if (fs.existsSync(path.join(distDir, 'index.html'))) {
path.join(distDir, 'index.html'),
path.join(distDir, '404.html')
);

fs.copyFileSync(
path.join(distDir, 'index.html'),
path.join(distDir, 'about.html')
);
Comment on lines +74 to +77

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This adds another fs.copyFileSync call to copy index.html. There is now duplicated logic for copying index.html to 404.html and about.html. Consider refactoring this to use a loop to improve maintainability and avoid repetition, for example:

['404.html', 'about.html'].forEach(file => {
  fs.copyFileSync(
    path.join(distDir, 'index.html'),
    path.join(distDir, file)
  );
});

}

console.log(`✅ Build ${allPosts.length} Posts Successfully`);
3 changes: 3 additions & 0 deletions public/about.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Hi, I'm Gapry. Nice to meet you! I love drinking coffee.

![](/assets/pic.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The image is missing alternative text (alt text). Alt text is crucial for accessibility, as it provides a description of the image for screen readers and is displayed if the image fails to load. Please add a descriptive alt text.

Suggested change
![](/assets/pic.png)
![A picture of Gapry](/assets/pic.png)

Binary file added public/assets/pic.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
13 changes: 13 additions & 0 deletions src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { vscDarkPlus } from 'react-syntax-highlighter/dist/esm/styles/prism';
import Analytics from './components/Analytics';
import NotFound from './pages/NotFound/NotFound';
import Home from './pages/Home/Home';
import Header from './components/Header/Header';
import Footer from './components/Footer/Footer';
import './styles/App.css';

Expand All @@ -32,6 +33,17 @@ export default function App() {
const pathClean = currentPath.replace(/\.html$/, '');
const parts = pathClean.split('/').filter(Boolean);

if (parts.length === 1 && parts[0] === 'about') {
fetch('/about.md')
.then(res => res.text())
.then(text => {
setContent(text);
setStatus('post');
})
.catch(() => setStatus('404'));
return;
}
Comment on lines +36 to +45

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The routing logic for the /about page is hardcoded. If you plan to add more static pages in the future, this approach will lead to a chain of if/else if statements, which is not easily maintainable. Consider abstracting this into a more scalable solution, for example by using a configuration object for static pages.

        const staticPages = {
          'about': '/about.md'
        };

        if (parts.length === 1 && staticPages[parts[0]]) {
          fetch(staticPages[parts[0]])
            .then(res => res.text())
            .then(text => {
              setContent(text);
              setStatus('post');
            })
            .catch(() => setStatus('404'));
          return;
        }


if (parts.length === 0 || (parts.length === 1 && parts[0] === 'index')) {
setStatus('home');
return;
Expand Down Expand Up @@ -73,6 +85,7 @@ export default function App() {
<>
<Analytics />
<div className="app-shell">
<Header />
{status === '404' ? (
<NotFound />
) : status === 'home' ? (
Expand Down
45 changes: 45 additions & 0 deletions src/components/Header/Header.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
.site-header {
width: 100%;
border-bottom: 1px solid #333;
padding: 1.5rem 0;
margin-bottom: 2rem;
}

.header-container {
display: flex;
justify-content: space-between;
align-items: flex-end;
max-width: 800px;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The max-width value of 800px is hardcoded here. This same value is used in other files like src/styles/App.css and src/pages/Home/Home.css. To improve maintainability and ensure consistency, it's best to define this as a CSS custom property (variable) in a global scope (e.g., in :root in src/styles/App.css) and reuse it across all relevant components.

margin: 0;
padding: 0;
}

.logo {
font-size: 2.0rem;
font-weight: bold;
text-decoration: none;
color: #fff;
line-height: 1;
}

.nav-menu {
list-style: none;
display: flex;
align-items: center;
gap: 20px;
margin: 0;
padding: 0;
}

.nav-menu a {
display: flex;
align-items: center;
text-decoration: none;
color: #aaa;
transition: color 0.2s;
line-height: 1;
}

.nav-menu a:hover {
color: #646cff;
}
22 changes: 22 additions & 0 deletions src/components/Header/Header.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import siteConfig from '../../data/config.json';
import './Header.css';

export default function Header() {
return (
<header className="site-header">
<div className="header-container">
<div className="header-left">
<a href="/" className="logo">
{siteConfig.siteName}
</a>
</div>

<nav className="header-right">
<ul className="nav-menu">
<li><a href="/about">About Me</a></li>
</ul>
</nav>
Comment on lines +8 to +18

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The links in the header (logo and nav-menu) use standard <a> tags, which will cause a full page reload on navigation. This is inefficient for a Single Page Application (SPA) and provides a poor user experience. You should implement client-side routing. Since you are not using a routing library, you can achieve this by:

  1. Adding onClick handlers to your links.
  2. In the handlers, call event.preventDefault() to stop the reload.
  3. Use window.history.pushState() to update the URL.
  4. Modify the App component to listen for popstate events and re-render the content accordingly.

</div>
</header>
);
}
2 changes: 1 addition & 1 deletion src/data/config.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"siteName": "Gapry's Technical Writing",
"siteName": "Gapry's Blog",
"analytics": {
"provider": "google-gtag",
"trackingId": "G-4DG3HSBT1T",
Expand Down
2 changes: 1 addition & 1 deletion src/pages/Home/Home.css
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
}

.home-title {
font-size: 2rem;
font-size: 1.5rem;
margin-bottom: 30px;
color: #ffffff;
}
Expand Down
Loading