モバイル用の投稿ダイアログ呼び出しボタンを追加 #128
Conversation
|
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (5)
- src/app/(menu)/_components/menu/elements/MobileOpenPostDialogButton.tsx (1)
- src/app/(menu)/layout.tsx (2)
- src/app/_components/post/PostDialog.tsx (2)
- src/app/_components/post/PostForm.tsx (1)
- src/app/_components/post/elements/Editor.tsx (2)
Files skipped from review due to trivial changes (2)
- src/app/(menu)/layout.tsx
- src/app/_components/post/elements/Editor.tsx
Review comments generated (4)
- Review: 3
- LGTM: 1
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| const renderablePaths = ['/home', '/search', '/notifications']; | ||
| const renderable = renderablePaths.includes(usePathname() as string); |
There was a problem hiding this comment.
usePathname()の戻り値がstringであることを保証するために、型アサーションを使用しています。しかし、この方法は安全ではありません。もしusePathname()がundefinedや他の非文字列値を返す場合、ランタイムエラーが発生します。代わりに、usePathname()の戻り値をチェックし、それがstringであることを確認する方がよいでしょう。
- const renderable = renderablePaths.includes(usePathname() as string);
+ const pathname = usePathname();
+ const renderable = typeof pathname === 'string' && renderablePaths.includes(pathname);There was a problem hiding this comment.
配列に含まれるかの確認だけなので、仮にundefinedや非文字列値が返されてもランタイムエラーは発生しないので問題ありません。
There was a problem hiding this comment.
@furutani-k あなたの観点は理解できます。JavaScriptでは、undefinedや非文字列値を配列のincludesメソッドに渡してもエラーは発生しません。しかし、このコードが将来的に変更され、usePathname()の戻り値が他の操作で使用される場合、型アサーションによる安全でないキャストは問題を引き起こす可能性があります。
そのため、私の提案は、現在のコードがエラーを引き起こす可能性があるというよりも、将来的な問題を予防するためのものです。ただし、あなたの指摘通り、現状では必須ではありません。
| export default function MobileOpenPostDialogButton() { | ||
| const [getOpenDialog, setOpenDialog] = useState(false); | ||
|
|
||
| const renderablePaths = ['/home', '/search', '/notifications']; | ||
| const renderable = renderablePaths.includes(usePathname() as string); | ||
| const { data: profile } = useProfile(); | ||
|
|
||
| return ( | ||
| <> | ||
| {profile && renderable && ( | ||
| <Box> | ||
| <StyledButton | ||
| variant="contained" | ||
| color="primary" | ||
| onClick={() => setOpenDialog(true)} | ||
| > | ||
| <Send /> | ||
| </StyledButton> | ||
|
|
||
| <PostDialog | ||
| fullScreen | ||
| open={getOpenDialog} | ||
| close={() => setOpenDialog(false)} | ||
| /> | ||
| </Box> | ||
| )} | ||
| </> | ||
| ); | ||
| } |
There was a problem hiding this comment.
useStateフックの変数名getOpenDialogは、関数またはメソッドを示唆していますが、実際にはブール値を保持しています。これは混乱を招く可能性があります。より適切な名前に変更することをお勧めします。
- const [getOpenDialog, setOpenDialog] = useState(false);
+ const [isDialogOpen, setDialogOpen] = useState(false);そして、この新しい変数名をコード内で適切に使用します。
- open={getOpenDialog}
+ open={isDialogOpen}
- onClick={() => setOpenDialog(true)}
+ onClick={() => setDialogOpen(true)}
- close={() => setOpenDialog(false)}
+ close={() => setDialogOpen(false)}There was a problem hiding this comment.
既存のコードがget*, set*での記述となっているため、それに合わせたコードとなっています。
There was a problem hiding this comment.
@furutani-k 既存のコードがget*, set*の形式で記述されているとのこと、理解しました。ただし、この場合にはgetOpenDialogが実際には状態（ブール値）を保持しており、関数やメソッドを示唆する名前となっています。これは混乱を招く可能性があります。
一般的に、get*やset*のプレフィックスは、値を取得または設定するための関数やメソッドに使用されます。一方、状態変数は通常、その状態を表す名前が付けられます。例えば、ダイアログが開いているかどうかを示すブール値であれば、isDialogOpenのような名前が適切です。
したがって、私の提案は、混乱を避けるためにも、この特定の状況ではget*, set*の命名規則から逸脱し、より明確な名前を使用することです。
There was a problem hiding this comment.
@furutani-k 取得される値は関数(所謂getter)ではなく、状態を表すのでopenやisOpenに名称変更する方向でお願いします。
既存の箇所も随時直していく感じで
| export default function MobileOpenPostDialogButton() { | ||
| const [getOpenDialog, setOpenDialog] = useState(false); | ||
|
|
||
| const renderablePaths = ['/home', '/search', '/notifications']; |
There was a problem hiding this comment.
@furutani-k
今後ユーザーページでも呟ける必要があるので、パス制限いらないかも？
DMや設定などボタン不要なパスを一覧にするほうが良さそう。
|
|
||
| return ( | ||
| <> | ||
| {profile && renderable && ( |
There was a problem hiding this comment.
@furutani-k ログインチェックにはuseAuthを使われるのが良いかと！
| return ( | ||
| <> | ||
| {profile && renderable && ( | ||
| <Box> |
There was a problem hiding this comment.
@furutani-k ここBoxで囲う必要がないような気がします。
| export default function MobileOpenPostDialogButton() { | ||
| const [getOpenDialog, setOpenDialog] = useState(false); | ||
|
|
||
| const renderablePaths = ['/home', '/search', '/notifications']; | ||
| const renderable = renderablePaths.includes(usePathname() as string); | ||
| const { data: profile } = useProfile(); | ||
|
|
||
| return ( | ||
| <> | ||
| {profile && renderable && ( | ||
| <Box> | ||
| <StyledButton | ||
| variant="contained" | ||
| color="primary" | ||
| onClick={() => setOpenDialog(true)} | ||
| > | ||
| <Send /> | ||
| </StyledButton> | ||
|
|
||
| <PostDialog | ||
| fullScreen | ||
| open={getOpenDialog} | ||
| close={() => setOpenDialog(false)} | ||
| /> | ||
| </Box> | ||
| )} | ||
| </> | ||
| ); | ||
| } |
There was a problem hiding this comment.
@furutani-k 取得される値は関数(所謂getter)ではなく、状態を表すのでopenやisOpenに名称変更する方向でお願いします。
既存の箇所も随時直していく感じで
| height: 100% !important; | ||
| vertical-align: top; |
There was a problem hiding this comment.
@furutani-k
MUIがheightをJSで制御してるのが厄介ですね。
可能な限り!importantは使用したくはないですが...
こちらでも触ってみます。
There was a problem hiding this comment.
Files selected (1)
- src/app/_components/post/PostDialog.tsx (2)
Files not summarized due to errors (1)
- src/app/_components/post/PostDialog.tsx (nothing obtained from openai)
Files not reviewed due to errors (1)
- src/app/_components/post/PostDialog.tsx (no response)
Review comments generated (0)
- Review: 0
- LGTM: 0
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
There was a problem hiding this comment.
Files selected (1)
- src/app/layout.tsx (2)
Files not summarized due to errors (1)
- src/app/layout.tsx (nothing obtained from openai)
Files not reviewed due to errors (1)
- src/app/layout.tsx (no response)
Review comments generated (0)
- Review: 0
- LGTM: 0
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
takecchi
force-pushed
the
feature/issues-112-mobile-post-form
branch
from
6531e8f
to
0c90ca6
Compare
takecchi
changed the title
takecchi
changed the title
|
#134 |
スタイル調整
Issue
変更内容
確認方法
(確認方法を記載してください)
Screenshot (任意)
Screencast.from.2023.12.14.19.51.15.webm
Summary by CodeRabbit
PostDialogコンポーネントにフルスクリーンプロパティを追加しました。これにより、投稿ダイアログがフルスクリーンモードで表示され、ユーザー体験が向上します。