-
Notifications
You must be signed in to change notification settings - Fork 0
ドキュメント一覧ページを作成する #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ドキュメント一覧ページを作成する #38
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
src/app/docs/page.tsx
Outdated
import { useState, useEffect } from "react"; | ||
import supabase from "../../lib/supabase"; | ||
|
||
interface Doc { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DBにあるテーブルの型はsupabaseの機能で生成したものを使ってください。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
型を supabase の機能で生成したものに変更しました🖊
src/app/docs/page.tsx
Outdated
const [totalPages, setTotalPages] = useState(0); | ||
const [loading, setLoading] = useState(true); | ||
|
||
const fetchDocs = async (page: number) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
テストしやすいようにページにはコンポーネントを置くだけの感じでお願いします〜。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/page.tsx にコンポネントを置くだけに変更しました🖊
cbcf4f8
to
c2d8a05
Compare
8be2d36
to
f33ed8c
Compare
src/app/docs/layout.tsx
Outdated
@@ -0,0 +1,9 @@ | |||
import SingleLayout from "@/components/layouts/SingleLayout"; | |||
|
|||
export default function DashboardLayout({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export default function DashboardLayout({ | |
export default function DocsLayout({ |
の方がいいかも?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DocsLayout に修正しました🖊
<li> | ||
<Link href="/dashboard">ダッシュボード</Link> | ||
</li> | ||
{links.map((link, index) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/components/DocList.tsx
Outdated
import supabase from "../lib/supabase"; | ||
import Pagination from "./Pagination"; | ||
|
||
interface Doc { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これって自分で手書き定義しているからsupabaseの機能で生成した型を使っていることにならないかな〜と思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type Doc = Tables<"docs">;
に変更しました🖊
https://supabase.com/docs/guides/api/rest/generating-types#type-shorthands
updated_at TIMESTAMP DEFAULT NOW(), | ||
user_id INT NOT NULL, | ||
last_updated_user_id INT NOT NULL | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
最終行に改行を挿入しました🖊
またエディタの設定で最後に改行されるようにしました⚙
@komagata さん、 |
ec03455
to
24dbfe9
Compare
これに対するcomponent test, e2e testの作成をお願いします。 |
24dbfe9
to
68e28dd
Compare
@kazuma-naka こちら、最後のコメントが僕になっているので、該当箇所が修正完了して、再度レビューが必要という状態になったらメンションで連絡お願いします。 今の状態はどういう状態ですか?(どちらがやり取りのボールを持っているのか) |
お疲れ様です。 |
77ef109
to
2facd41
Compare
545430f
to
1a6c1ed
Compare
@@ -0,0 +1,53 @@ | |||
"use client"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
App Routerとしては、できるだけ (client側の) hooksに依存せずに、サーバーで処理するのが基本なのかなと思います。
webで見つかるサンプルはclientのほうが多いようですね。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ページネーションが hooks なので、そこを切り出して use client にするのがいいかも 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
逆ではなく、できるところは server で処理した方が良いかなという意見でした!
ページネーションの state とかは use client じゃないとできない気がするのでその部分は client component になると思うんですが、他のところは server にできると良いのかな〜という気持ちです!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ページネーションの state とかは use client じゃないとできない気がするので
すみません、こちら意味が理解できなかったのですが、もう少しご説明いただければありがたいです〜。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ページネーションですが、
- server側でページネーションし、指定されたページを返す
- server側で全部返しておいて、clientでページネーションする
の後者の意味かなと思います。
普通にserverで実装すると前者になりそうですが、ページの行き来や、フィルタ条件を動的に変えるなら後者になるのかなと思います。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hotpepsi さんの記載の通りです!!
返信遅れました 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
僕も確認させて頂きました。OKです〜🙆♂️
Issue
1回目の提出
概要
Docs
ページを作成Docs
ページにリストとページャーを設置変更確認方法
feature/create-docs-list
をローカルに取り込むnpm run dev
でローカル環境を立ち上げる変更前
変更後
top page
docs
概要
Docs
ページを作成Docs
ページにリストとページャーを設置useDocs
hook の作成itemsPerPage
で表示するドキュメント数を設定できるようにするBootCamp のドキュメント一覧と同じような仕様を心がけました。
変更確認方法
feature/create-docs-list
をローカルに取り込むnpm run dev
でローカル環境を立ち上げる変更前
変更後
top page
docs
修正前
コンポネントテストの実装
モックの Docs が表示されるかテスト
最初のページ、最後のページ、現在のページから一つ前、後のページに遷移するボタンが表示されているか、クリックすると遷移するかテストする。
## E2E テストの実装- doclist.test.tsSupawright でテスト用の docs データを作成。created_at
順で新しいものからソートされるようにする。テストデータにはテスト時の時間を渡すことで一番初めにテストデータが表示されるようにする。このPRではE2Eテストは実装しない