Conversation
src/components/MediaView.tsx
Outdated
| assets: Pick<Asset, "id" | "type" | "url">[]; | ||
| className?: string; | ||
| }) => { | ||
| if (props.assets.length === 0) return <></>; |
There was a problem hiding this comment.
何も返さない時の値、Reactの推奨は空のフラグメントではなくnullなのでこっちでお願いしたいです🫠
https://ja.react.dev/learn/conditional-rendering#conditionally-returning-nothing-with-null
|
|
||
| import styles from "./CodeInline.module.css"; | ||
|
|
||
| const CodeInline = (props: React.HTMLAttributes<HTMLElement>) => { |
There was a problem hiding this comment.
propsとしてchildren以外も受け取れるのにchildrenしか適用されない状態はちょっとわかりづらそうです。propsの型をComponentPropsとかにしてあげて、スプレッド構文で渡してあげるのはどうでしょう
There was a problem hiding this comment.
あ～確かに、そちらの方が明確で良さそうですね
| import { Title, TitleOrder } from "@mantine/core"; | ||
|
|
||
| const Heading = ( | ||
| props: React.HTMLAttributes<HTMLHeadingElement> & { |
| style={{ | ||
| position: "relative", | ||
| paddingRight: "1.5rem", | ||
| }} |
There was a problem hiding this comment.
細かいですがstyleは使わずclassNameを使うようお願いします🙏
There was a problem hiding this comment.
ここなんですが、一度classNameで指定しようとした所スタイルが思ったように適用されなかったんですよね…
There was a problem hiding this comment.
修正のタイミングですぐ直せそうなら直して、ダメそうだったらコメント書き残しておきます 🫠
There was a problem hiding this comment.
ディレクトリ名、先頭にアンダースコア入れてる理由あったりしますか？
appディレクトリ配下の場合はルートとして認識させないようにする必要があったので付けてましたが、もし特に理由がなければ消しちゃって大丈夫です👍
There was a problem hiding this comment.
「Markdown.tsxの中でのみ使用する子孫コンポーネントである」という意図でアンダースコアを付けていましたが、まああまり一般的ではないですしそこまで必要無さそうですかね？
There was a problem hiding this comment.
ちょっと関係性をそれで伝えるのは分かりづらいので、もし意図がそれだけだったら不要です！むしろアンダースコアを外せばMarkdown.tsxとmarkdown/がエディタ上で近くなるので見やすそう🤔
There was a problem hiding this comment.
確かに！それは盲点でした…
であればアンダースコア無しに修正しておきます 🙇♂️
|
@newt239 修正しました！再レビューお願いします |
2 participants
関連 Issue(s)
変更内容
スクリーンショット
各種要素の表示 (拡大推奨)
スマートフォン表示 (1)
スマートフォン表示 (2)
確認手順