Apply the approved preview design to the slide page - #421
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| <Link | ||
| aria-label={mobileSize ? "スライドショーを開始" : undefined} | ||
| css={[ | ||
| headerButtonStyle, | ||
| headerGrayButtonStyle, | ||
| mobileSize && headerIconButtonStyle, | ||
| ]} | ||
| to={presentPath} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| {!mobileSize && "スライドショーを開始"} | ||
| <MaterialSymbol>slideshow</MaterialSymbol> | ||
| </Link> | ||
| <button | ||
| aria-label={mobileSize ? "スライドを投稿する" : undefined} | ||
| css={[headerButtonStyle, mobileSize && headerIconButtonStyle]} | ||
| disabled={!isSlidePublishable} | ||
| onClick={handlePublish} | ||
| > | ||
| {!mobileSize && "スライドを投稿する"} | ||
| <MaterialSymbol>publish</MaterialSymbol> | ||
| </button> |
There was a problem hiding this comment.
直しました（f36199a）。
<button> だけが UA スタイルのフォント（Arial 13.33px）のままで、フォントを継承する <a> より低くなっていました（1920px で 33px と 40.8px）。headerButtonStyle に font: inherit を足して、どちらも 40.8px に揃えています。375px はもともと両方 48×48 です。
このスタイルは記事の「記事を投稿する」ボタンでも使っているので、そちらも 16px・40.8px に変わります（PR 本文に追記しました）。
🤖 Generated by Claude Code
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
DeguchiHiroki
left a comment
DeguchiHiroki
left a comment
There was a problem hiding this comment.
AI による一次レビューです（must: 0件 / suggestion: 0件 / question: 0件）。
最終的なレビュー判断は @DeguchiHiroki が行います。
Preview のスライド画面に #413 のデザインを反映する差分です。ヘッダーのボタン構成（プレゼン開始・投稿）、スピーカーノート表示、?present=1 のフッター削除、Colors.surfaceVariant の変数名修正、投稿 API の追加を確認しました。差分に現れた範囲では要件との齟齬・アクセシビリティ・実装上の問題は見当たりません。
tomoasleep さん自身の AI Review（本文中の button 高さ不一致・タグ chip 背景色の疑問）はいずれもコード側で解消済み、または PR 本文で開示済みと確認しています。CI は全チェック成功、承認待ちのみの状態です。
| <Link | ||
| aria-label={mobileSize ? "スライドショーを開始" : undefined} | ||
| css={[ | ||
| headerButtonStyle, |
There was a problem hiding this comment.
headerButtonStyleのhover時に textDecoration: "none", を追加したいです。
There was a problem hiding this comment.
追加しました（50e2d28）。
hover 時に「スライドショーを開始」（<a>）に下線が付いていたのが消えることを、Playwright で確認しています（text-decoration-line が underline → none）。Before / After のスクリーンショットは PR 本文の「ヘッダーのボタンリンクに hover で下線が付かないようにした」に貼りました。<button> のボタンは、もともと下線が付かないので変化はありません。
🤖 Generated by Claude Code
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Qiita defines --color-surfaceVariant, so var(--color-surface-variant) resolved to nothing and left the background transparent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow the approved preview design: a 1px divider border replaces the shadow, and speaker notes appear under each page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The publish button posts through the new slides API. On mobile both collapse into 48px icon buttons, as in the approved design. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The publish <button> kept the user agent font (Arial 13.33px) while the slideshow <a> inherited the page font, so the button was 33px tall against the link's 40.8px on desktop. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f36199a to
1d283d4
Compare
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
tomoasleep
left a comment
tomoasleep
left a comment
There was a problem hiding this comment.
🤖 AI Review 🤖
問題はありません。
その他確認した項目
何を確認したか
- PR: #421
- diff: main...apply-design-to-slide-feature
- CI state: success（6 checks: check_test_execution_conditions, lint, test x3, build）
- 既存レビュー: 行コメント2件（いずれも解消済み）、トップレベルコメント1件（must/suggestion/question とも0件）。DeguchiHiroki が現在の head commit を承認済み
見たが問題にしなかった観点
- Header.tsx のボタンサイズ不一致の指摘は、
headerButtonStyleにfont: inheritを追加する形で解消している（src/client/components/Header.tsx:294付近）。font: inheritがfontWeight: Weight.boldより前に定義されているため、shorthand のfontが個別指定のfontWeightを上書きすることはない。 - hover 時に下線が残る指摘も、
headerButtonStyleの hover にtextDecoration: "none"を追加する形で解消している（src/client/components/Header.tsx:310付近）。 - トップレベルレビューが挙げていたボタン高さ不一致・タグ chip 背景色の見た目変化は、いずれもレビュー投稿より前のコミットで解消済み、またはタグ chip の見た目変化は PR 本文で開示済みであることを diff とコミット順序で確認した。追加対応は不要と判断する。
slidesUpdate（サーバー側 API）は、404 判定・basename でのディスパッチ・publish 失敗時に{ success: false }を返す構造まで、既存の記事用 API と一行単位で同じ形になっている。- スライド用ヘッダーの publish ハンドラは、記事用ヘッダーにある例外時のフォールバック処理を持たないが、
navigate()呼び出しの構成上その処理がなくても実害はなく、記事側との既存の実装差分を踏襲したものである。 - 新しいボタンのアイコンとラベルの付け方は、既存の記事用ヘッダーと同じパターンになっており、アクセシビリティ上の問題は見当たらない。
Header.tsxやMarpSlideViewerはもともとコンポーネント単体テストを持たず、今回もサーバー側 API テストと Playwright での確認に寄せている構成で、既存の踏襲パターンから外れていない。- 投稿ボタンの活性条件が特定のフラグを見ていない点も、記事側の同等ロジックと挙動が揃っており、今回新たに生まれたギャップではない。
補足
CI は全チェック成功、merge state は CLEAN（コンフリクトなし）で、DeguchiHiroki が現在の head commit を承認済み。マージ判断は人間に委ねる。
head: 50e2d28
2 participants
What
#413 で確定した Preview のデザイン（
design/pages/slides/slides.pen）を、qiita previewのスライド画面（/slides/[id]）に反映する。Colors.divider）に変えるN / 3）を消すspeaker_noteが空のページには出さない）breakpoint.S以下）では、どちらも 48×48 のアイコンボタンにするPOST /api/slides/:idを追加する。記事のPOST /api/items/:idと同じ形で、SlideFileSystemRepo#publishSlideを呼ぶ?present=1）のフッター（罫線・タイトル・ページカウンタ）を消すColors.surfaceVariantが参照する CSS 変数名を、Qiita の CSS が定義している名前に直すサイドバー・一覧・新規作成・スライド情報・バリデーション警告・404・フロントマター不正の各状態は、
.pen側も実装をそのまま写した状態なので、変更していない。デザインから意図的に外したもの
?present=1 - start slideshow modal state（「スライドショーを開始する」モーダル）は入れていない。このフレームは #413 のレビューでの「今後 qiita.com 同様プレゼン画面選択モーダルを追加したい」を受けて描かれたもので、プレゼンタービューの表示先を選ぶ UI が含まれている。ただ、プレゼンタービューそのものに対応するフレームがデザインに無い。いまモーダルだけを置くと、選んでも機能しない選択肢が残ってしまう。そのため、この PR の「スライドショーを開始」は、これまでどおり?present=1を新しいタブで開く。モーダルは別 PR で扱いたい。記事のタグの見た目も変わる
Colors.surfaceVariantは記事のタグの chip（Article.tsx）でも使われている。これまで背景は透明だったが、この PR からgray20の背景が付く（items.penは透明のまま描かれている）。デザインシステム本来の値に戻る変更だが、見た目は変わる。ヘッダーのボタンがフォントを継承するようにした
「スライドを投稿する」の
<button>だけが UA スタイルのフォント（Arial 13.33px）のままで、ページのフォントを継承する「スライドショーを開始」の<a>より低くなっていた（1920px で 33px と 40.8px）。headerButtonStyleにfont: inheritを足して揃えた。headerButtonStyleは記事の「記事を投稿する」ボタンでも使っているので、こちらも同じくページのフォント（16px）になり、高さ 40.8px に変わる。下の「Screenshot」の各画像はこの修正の前に撮ったもので、投稿ボタンが低く写っている。
ヘッダーのボタンリンクに hover で下線が付かないようにした
「スライドショーを開始」は
<a>なので、hover するとページ全体のリンクのスタイルで下線が付いていた。headerButtonStyleの hover にtextDecoration: "none"を足して消した。<button>の「スライドを投稿する」「記事を投稿する」は、もともと下線が付かないので見た目は変わらない。How
スピーカーノートは1要素を1段落で出す
デザインのスピーカーノートの本文は代表値で、複数ノートや改行の扱いは決まっていなかった。そこで
speaker_note: string[]の1要素を1段落にして、改行はwhite-space: pre-wrapでそのまま出すことにした。ノートが無いページには、ブロック自体を出さない。投稿ボタンを押せる条件は記事と同じにする
modified && error_messages.length === 0のときだけ押せる。未投稿のスライドはミラーが無いのでmodifiedが true になり、押せる。リモートより古い場合（is_older_than_remote）は、記事と同じく confirm で確認してから上書きする。そのためSlidesShowViewModelにmodified/is_older_than_remoteを追加した。「投稿する」という短いラベルは使わない
デザインでは 375px をアイコンボタンだけにしていて、短縮ラベルは採用していない（#413 の本文に記載）。そのため、記事の
Headerにある投稿するの分岐は、スライドには入れていない。Why
なぜ
Colors.surfaceVariantの定義を直すのかスピーカーノートの背景は、デザインでは
surface-variant（Light ではgray20）になっている。ところがvariables.tsはvar(--color-surface-variant)を参照していて、Qiita の CSS が定義しているのは--color-surfaceVariantのほうだった。このままでは背景が透明になるので、変数名を Qiita の CSS に揃えた（#413 の「相談したいこと 2」で挙がっていた件）。動作確認
自動テスト
yarn lint/yarn build/yarn test（25 suites, 227 tests）がすべて成功。src/server/api/slides.test.tsを追加し、次の2点を確認している。POST /api/slides/:idの投稿（basename 指定・uuid 指定）・404・投稿失敗GET /api/slides/:idがmodified/is_older_than_remoteを返すことPlaywright
Qiita API の代わりに stub サーバーを立て、ビルド済みの CLI（
node dist/main.js preview）をそこへ向けて確認した。stub が再現しているのは/api/v2/slide_previews//api/v2/slides//api/v2/authenticated_user/slidesなど。slide_previewsは Marp の出力と同じ形（<svg data-marpit-svg>とspeaker_note: string[]）を簡易的に作っているだけなので、実際の Marp のレンダリング結果と組み合わせた確認はしていない。/slides/[id]を開く?present=1が新しいタブで開く?present=1を開くPOST /api/v2/slidesが送られ、「スライドが投稿されました」が出て/slides/<uuid>に遷移する。遷移後は差分なしになり、ボタンは disabled になる。frontmatter にid/updated_atが書き戻され、marp/themeは残るtrace.zip も全ケースで生成したが、
gh --attachはメディア以外のファイルに対応していないので、ここには貼っていない。Screenshot
?present=1Refs
🤖 Generated with Claude Code