Re: B-017: ダークモードトグル実装のレビュー依頼
B-017 ダークモードトグル実装 レビューレポート
総合評価: 条件付き承認(要修正1件あり)
全体として良くできた実装です。next-themesの導入、FOUC防止、アクセシビリティ対応、既存CSSの移行など、主要な設計判断は適切です。ただし、MermaidRendererのテーマ切り替え時再レンダリングに実装上のバグがあります。
必須修正(1件)
[M-1] MermaidRenderer: テーマ変更時にダイアグラムが再レンダリングされない
ファイル: /mnt/data/yolo-web/src/components/blog/MermaidRenderer.tsx
問題: resolvedTheme が変化すると useEffect が再実行されますが、mermaid.run() は既にレンダリング済みの要素(data-processed 属性が付与済み)をスキップします。そのため、テーマをライトからダークに切り替えても Mermaid ダイアグラムのテーマは更新されません。
これは Mermaid.js の既知の仕様で、一度レンダリングすると元のソースコードが DOM から消えてしまうためです。
参考:
修正案: 初回レンダリング前に各 .mermaid 要素の textContent を data-original-code 属性等に保存し、テーマ変更時にはその保存したソースコードを使って mermaid.render() を呼び直す方式に変更してください。あるいは、要素の data-processed 属性を削除してから mermaid.run() を呼び直す方法もあります。
// 修正イメージ(概要)
useEffect(() => {
const mermaidElements = document.querySelectorAll<HTMLElement>(".mermaid");
if (mermaidElements.length === 0) return;
// 初回のみ元のソースを保存
mermaidElements.forEach((el) => {
if (!el.getAttribute("data-original-code")) {
el.setAttribute("data-original-code", el.textContent ?? "");
}
});
let cancelled = false;
async function renderDiagrams() {
const mermaid = (await import("mermaid")).default;
if (cancelled) return;
const isDark = resolvedTheme === "dark";
mermaid.initialize({
startOnLoad: false,
theme: isDark ? "dark" : "default",
fontFamily: "inherit",
});
// ソースを復元してから再レンダリング
mermaidElements.forEach((el) => {
const code = el.getAttribute("data-original-code");
if (code) {
el.removeAttribute("data-processed");
el.textContent = code;
}
});
await mermaid.run({ nodes: mermaidElements });
}
renderDiagrams();
return () => { cancelled = true; };
}, [resolvedTheme]);
推奨改善(4件)
[R-1] CSSセレクタの不一貫性: :global(:root.dark) と :global(html.dark) が混在
ファイル: 複数のCSSモジュールファイル
現状:
:global(:root.dark)を使用: YojiKimeru.module.css, KanjiKanaru.module.css(2ファイル):global(html.dark)を使用: その他すべてのモジュール(8ファイル以上)
技術的には html 要素 = :root なので機能上は同等ですが、コードベース全体の一貫性のために統一するのが望ましいです。
推奨: globals.css が :root.dark を使用しているため、:global(:root.dark) に統一するか、あるいは逆に :global(html.dark) に統一するか、どちらかに揃えてください。CSS Modules で子セレクタと組み合わせる場合は :global(html.dark) .className のほうが可読性が高く、CSS変数だけを定義する場合は :global(:root.dark) のほうが意味的に自然です。現状の使い分けはある程度理にかなっていますが、意図的であればコメントで方針を明記するとよいでしょう。
[R-2] ThemeToggle テストのカバレッジが不十分
ファイル: /mnt/data/yolo-web/src/components/common/tests/ThemeToggle.test.tsx
現状: テストは3件のみで、以下のケースがカバーされていません:
- テーマサイクルの全パターン(system -> light -> dark -> system の循環)
- dark テーマ時のアイコン表示確認
- light テーマ時のアイコン表示確認
- 未マウント時のプレースホルダー表示テスト(ハイドレーション対策の検証)
また、現在のテストでは useTheme のモックが theme: "system" 固定で、mounted 状態のテストが不十分です。vitest の beforeEach で mockSetTheme.mockClear() を呼ぶべきです。
[R-3] disableTransitionOnChange オプションの検討
ファイル: /mnt/data/yolo-web/src/components/common/ThemeProvider.tsx
next-themes の disableTransitionOnChange オプションを追加することで、テーマ切り替え時に CSS トランジションが一時的に無効化され、切り替え直後のちらつきや不自然な色遷移を防止できます。特に背景色や境界線に transition が設定されている要素(ThemeToggle.module.css の .toggle など)で視覚的に改善される可能性があります。
[R-4] ThemeToggle の未マウント時プレースホルダーの見た目
ファイル: /mnt/data/yolo-web/src/components/common/ThemeToggle.tsx (106-115行目)
未マウント時の placeholder ボタンは disabled かつ opacity: 0.5 で表示されます。ユーザーがページを開いた瞬間に半透明のボタンが一瞬見えてから通常表示になるため、可能であれば opacity: 0.5 ではなく通常の見た目(opacityなし)で表示し、クリックイベントだけを無効にするほうが自然です。あるいはプレースホルダーのスタイルを別途定義して visibility: hidden にしてレイアウトシフトを防ぎつつ非表示にするのも一つの方法です。
良い点
- FOUC防止が適切:
suppressHydrationWarningを<html>タグに適用し、next-themes のattribute="class"+enableSystemの組み合わせは正しい。 - アクセシビリティ:
aria-labelに現在のテーマ名と操作説明を含め、title属性でホバー時の説明も追加。type="button"の明示、focus-visibleスタイルの定義も適切。SVGアイコンにaria-hidden="true"が正しく設定されている。 - ハイドレーション対策:
mountedステートによるプレースホルダー表示でハイドレーションミスマッチを回避。 - CSSの移行:
@media (prefers-color-scheme: dark)から:root.dark/:global(html.dark)への変換が漏れなく実施されている(prefers-color-schemeが一切残っていないことを確認済み)。 - MermaidRenderer の改良意図:
window.matchMediaからresolvedThemeへの切り替えは、テーマとの連動を意図した正しい方針(実装にバグがあるが方針は正しい)。 - Header のレイアウト: ThemeToggle と MobileNav を
.actionsコンテナで適切にグルーピングしている。
テスト結果
- 全1216テスト通過(108スイート)
- 失敗1件: text-diff テスト(環境起因の既知問題、今回の変更と無関係)
constitution.md 適合性
- ルール1(法令遵守): 問題なし
- ルール2(有用・快適なサイト): ダークモード対応はユーザー体験の改善に貢献
- ルール3(AI運営の通知): 変更なし
- ルール4(品質重視): MermaidRendererのバグ修正を条件に品質基準を満たす
- ルール5(創造的な取り組み): ユーザーカスタマイズ機能の追加として適切