Skip to content
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

メニューを新規作成モーダルを実装しました。 #55

Merged
merged 78 commits into from
Apr 4, 2023

Conversation

hata0
Copy link
Collaborator

@hata0 hata0 commented Oct 20, 2022

概要

#47 の内容を実装しました。

デバッグ項目

  • modalは正常に動作しています。
  • レスポンシブできています。
  • コンソールに警告はありません。
  • 外観に大きな違和感はありません。

スクリーンショット

2022-10-20.15-11-20.mp4

参考 URL

Self-Checking points 🚨

レビューを依頼する前に必ず確認してほしいポイントです。
一般的な項目を上げているので、プロジェクト毎に必要なポイントがあれば各リポジトリで追加してください。

共通 (命名)

  • data, info, value などの汎用的で抽象度の高い変数名を使っていない → 参考
  • 配列には末尾に s をつけて複数形にするか xxxList と命名することで配列であることを明確にしている → 参考
  • マジックナンバーは極力存在せず、定数を用いて表現されている 参考
  • パッと見で何をやってるかわからないような処理の結果を 説明変数 している → 参考
  • 複雑な条件式の結果を 要約変数 を適用している → 参考

共通 (処理系)

  • 早期リターン(ガード節) を適用することで条件分岐の簡略化している → 参考
  • if 文では「調査対象」を左側に、「比較対象」を右側に配置している → 参考
  • 値のパターンによって分岐する場合は is/else文 ではなく、switch文 を使う → 参考

共通 (コメント系)

  • コメントには適切なアノテーションコメントが記載されている → 参考

JavaScript (命名)

  • 変数名, 関数名, プロパティ名 は ローワーキャメルケース になっている → 参考
  • クラス名, オブジェクト名 は パスカルケース になっている → 参考
  • 定数名は スネークケース になっている → 参考

JavaScript (処理系)

  • 変数の宣言には極力を const を利用している → 参考
  • 文字列の合成にはテンプレートリテラルを利用している → 参考
  • ループ処理は 通常for文, for-in, forEach などは極力利用せず、高階関数を利用している → 参考
  • ==(等価演算子) は使わず ===(厳密等価演算子) を利用している → 参考

TypeScript (命名)

  • インターフェース, タイプ, Enum の命名は パスカルケース になっている → 参考

React (処理系)

  • コンポーネントの変数名は パスカルケース になっている → 参考
  • イベント処理のロジックは JSX 内に直接かかず、コールバック関数として JSX 外で定義する → 参考
  • コンポーネントに子要素がない場合は 自己終了タグ を利用する → 参考
  • attribute にブール値を直接記載する場合は省略形を利用する → 参考
  • attribute に文字列を直接記載する場合は波括弧を利用しない → 参考

共通 (その他)

  • フォーマット差分などは PR 上に一言 インラインコメント を付けて、レビュワーが省エネできるように → 参考

Copy link
Contributor

@umiteru2004 umiteru2004 left a comment

Choose a reason for hiding this comment

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

そろそろ完成!

Copy link
Contributor

@umiteru2004 umiteru2004 left a comment

Choose a reason for hiding this comment

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

最後!

umiteru2004
umiteru2004 previously approved these changes Mar 10, 2023
Copy link
Contributor

@umiteru2004 umiteru2004 left a comment

Choose a reason for hiding this comment

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

お疲れ様!

Copy link
Member

@kubo-hide-kun kubo-hide-kun left a comment

Choose a reason for hiding this comment

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

見ましたー

Copy link
Contributor

@umiteru2004 umiteru2004 left a comment

Choose a reason for hiding this comment

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

Approve忘れてた

@umiteru2004 umiteru2004 merged commit fc3cf45 into develop Apr 4, 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