Skip to content

Comments

Error failed loading#94

Open
LLstream wants to merge 3 commits intodevelopmentfrom
error-failed-loading
Open

Error failed loading#94
LLstream wants to merge 3 commits intodevelopmentfrom
error-failed-loading

Conversation

@LLstream
Copy link

@LLstream LLstream commented Nov 6, 2023

No description provided.

@LLstream LLstream linked an issue Nov 6, 2023 that may be closed by this pull request
@LLstream LLstream requested review from a01sa01to and sor4chi November 6, 2023 09:13
Copy link
Member

@sor4chi sor4chi left a comment

Choose a reason for hiding this comment

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

レビュー遅れてすみません、数点お願いします!

function App() {
const [posts, setPosts] = useState([]);
const [isLoading, setIsLoading] = useState(false);
const [LoadFailed, setLoadFailed] = useState(false);
Copy link
Member

Choose a reason for hiding this comment

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

細かい指摘になってしまいますが、JavaScriptでは変数や関数の命名はlowerCamelCaseで行うような慣習があります。
また、Booleanの変数にはprefixにisやcanなどをつけるような慣習があります。変数を見た時に大体どういうことを想定した変数なのかを判断するために命名を規則づけているんです。

今回だとisLoadFailed / setIsLoadFailedのような命名にするのが良いと思います!

Comment on lines +36 to +41
<Typography textAlign="center"
variant="h6"
sx={{ color: 'error.main' }}
>
{LoadFailed ? "読み込みに失敗しました": ""}
</Typography>
Copy link
Member

Choose a reason for hiding this comment

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

この表示はいいと思います、確かに失敗した時にエラーメッセージ出るのはいいですよね。
ただ、LoadFailedがfalseの時にから文字列が返されることから、エラーでない場合は空の<Typography>が存在してしまうことになります。

なのでTypography全体を出し分けるようにすると良いと思います。

Suggested change
<Typography textAlign="center"
variant="h6"
sx={{ color: 'error.main' }}
>
{LoadFailed ? "読み込みに失敗しました": ""}
</Typography>
{LoadFailed && (
<Typography textAlign="center"
variant="h6"
sx={{ color: 'error.main' }}
>
読み込みに失敗しました
</Typography>
)}

Copy link
Member

Choose a reason for hiding this comment

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

  • ちなみに&&演算子は、左オペランド(今回だとLoadFailed)がtrueの時に右オペランド(今回だとTypography)を返す、falseの時はfalseを返すという挙動をします
  • Reactではfalseを埋め込むと存在が消されるのでうまく動くというわけです。

ここについてわからなかったら言ってください!

@LLstream LLstream closed this Nov 10, 2023
@a01sa01to
Copy link
Member

???

@a01sa01to a01sa01to reopened this Nov 10, 2023
@LLstream
Copy link
Author

間違えて押しちゃっててしかも気づいてませんでした
申し訳ないです…

@a01sa01to
Copy link
Member

あるある

1 similar comment
@sor4chi
Copy link
Member

sor4chi commented Nov 10, 2023

あるある

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