Skip to content

チャンネルヘッダーを実装#43

Open
Meiryo7743 wants to merge 31 commits intomainfrom
feat/channel-header
Open

チャンネルヘッダーを実装#43
Meiryo7743 wants to merge 31 commits intomainfrom
feat/channel-header

Conversation

@Meiryo7743
Copy link
Contributor

関連 Issue(s)

変更内容

各チャンネルのページにおいて,ヘッダーやページのタイトルを Digichat ではなくチャンネル名を表示するようにしました。

スクリーンショット

Before After
Before After

確認手順

  1. pnpm run dev を実行し,開発用サーバーを立ち上げる。
  2. 開発用サーバ上で任意のチャンネル (/channels/[channel_id]/) へアクセスする。
  3. そのチャンネルのページにおいて,以下の 2 点が正しく表示されているかを確認する。
    • ヘッダーにチャンネル名が表示されている。
    • ページタイトルが Digichat > チャンネル名 のような形式になっている。

@Meiryo7743 Meiryo7743 requested a review from newt239 March 2, 2025 12:23
@Meiryo7743 Meiryo7743 self-assigned this Mar 2, 2025
});

// channelName に書き換える処理だけを実行したいので,空の JSX を返す
return <></>;
Copy link
Member

Choose a reason for hiding this comment

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

ChannelHeaderコンポーネントを作るのであればページタイトルの更新処理もこっちに移動させてしまったほうが可読性が高い気がしますがどうでしょう?逆バケツリレー的なことをしていることもあって、余計に流れが追いづらいかもです
今後コード量が増えてくることを考えると、やはりAppShell.tsxは遠いのでこっちに処理をまとめてしまったほうが綺麗な気がします

Copy link
Contributor Author

Choose a reason for hiding this comment

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

現状だと,AppShell において,ヘッダー内にナビゲーションメニューのトグルが実装されています。このため,ヘッダーを書き換えると,もれなくトグルが消滅してしまうようです。ほかにも,Server Components と Client Components が混在しているため,下手にいじると Hydration に起因するエラーに見舞われる難しいところもあります。

こうした背景から,チャンネル名(ページタイトル)をヘッダーに表示するためには,Client Components として localStorage 経由で子たる ChannelHeader から親たる AppShell へのバケツリレーで伝搬させる強引な方法を採らない限り,実装不可能な気がしています。

良さげな打開案ってありますかね……?

Copy link
Member

Choose a reason for hiding this comment

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

ハンバーガーメニューは仮置きだったのでなくすのはありです。特にプロダクトの性質上基本的にチャンネルページ以外のページが存在しないので、そこにヘッダーを置いて左サイドバーを表示させるボタンを配置させても良さそうです。
ただこれ、個人的にはヘッダーを二重にするのが良いかなと思っていました。Discord, Mattermost, Slackいずれも二重のヘッダーを採用しているので、とりあえずAppShellのヘッダーは以前の状態を維持しつつ、新しくチャンネルページにabsoluteとかで配置するのはどうでしょう

@saka-naname
Copy link
Contributor

saka-naname commented Jun 11, 2025

以下の変更を加えました。

  • 最新のmainブランチから変更をマージ (コンフリクトを解消)
  • 現在開いているチャンネルのデータをContextを通して受け渡しできるように
  • チャンネルのヘッダーを2段にし、モバイルのときチャンネルのタイトルを表示
  • チャンネルページのタイトルを generateMetadata で変更するように
    • useDocumentTitle では特定条件において正しくページタイトルが反映されなかったため

image

image

@saka-naname saka-naname requested a review from newt239 June 11, 2025 11:33
position: fixed;
top: 3rem;
z-index: 10;
width: 100%;
Copy link
Member

Choose a reason for hiding this comment

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

sidebarがあるとき横が突き抜けてしまっているようです!

スクリーンショット 2025-06-13 12 57 58

Copy link
Contributor

Choose a reason for hiding this comment

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

修正します!

Comment on lines +22 to +24
include: {
members: true,
},
Copy link
Member

Choose a reason for hiding this comment

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

これは不要な気がします👀

Suggested change
include: {
members: true,
},


if (!context)
throw new Error(
"useCurrentChannel must be used within a <CurrentChannelProvider>"
Copy link
Member

Choose a reason for hiding this comment

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

nits-badge

細かいんですがエラーメッセージは日本語のほうが良いかもです
(分かりやすいというのもありますが、自分で書いたエラーなのかライブラリがthrowしたエラーなのかの区別がつきやすい)

@saka-naname
Copy link
Contributor

上記3項目について修正しました!

Copy link
Member

@newt239 newt239 left a comment

Choose a reason for hiding this comment

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

LGTMeow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

チャンネルヘッダーの実装

3 participants