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

フロントエンド側の実装&完成 #13

Open
wants to merge 29 commits into
base: develop
Choose a base branch
from

Conversation

imaimai17468
Copy link
Contributor

@imaimai17468 imaimai17468 commented Mar 29, 2023

WAHT

  • フロントエンド側でのTODOアプリの実装をした
  • 一旦完成

HOW

  • バックエンド

    • main.go内のメソッドの処理を修正
    • corsの設定を追加
  • フロント

    • バックエンドとの繋ぎこみ
    • 必要なコンポーネント実装
    • ページ実装
    • api送受信

TEST

  • make build してね

  • バックエンドのコードチェック

  • フロントエンドのコードチェック

  • タスクが追加できるか

  • タスクが編集できるか

  • タスクが削除できるか

  • スタイルの崩れがないか

@imaimai17468 imaimai17468 self-assigned this Mar 29, 2023
@imaimai17468 imaimai17468 changed the base branch from main to develop March 29, 2023 15:46
Copy link
Contributor

@ryuseifujisaki ryuseifujisaki left a comment

Choose a reason for hiding this comment

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

動作等は問題ないです。
コードだけ確認したらmergeで良さそうです

Copy link

@YushiroDodo63 YushiroDodo63 left a comment

Choose a reason for hiding this comment

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

レビューしたので確認お願いします!

あと、 それぞれのコンポーネントに type ファイルを配置しとる理由ってどーゆー理由だったりする??

`}
{...rest}
className="bg-background text-secondary rounded-md shadow-md p-2 hover:bg-orange-600 transition-all"
onClick={props.onClick}

Choose a reason for hiding this comment

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

ここって props.onClick が渡されなかった場合(undefinedだった場合)ってどうなる?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JSXのbuttonって、undefinedの場合も加味されてるので、基本的にはなにも起きないんですが、それはそれとしてundeifnedのチェックは必要なので修正ました!

[fix] props.onClickのundefined処理

Comment on lines 9 to 13
const [formData, setFormData] = useState<SubmitTaskData>({
task_name: "",
content: "",
user_name: "",
});

Choose a reason for hiding this comment

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

ここはキャメルケースの方が好ましいかも!
backend も スネークケースになっとるならそれも治すように @ryuseifujisaki とかに連絡したほうがいいかもっすね、確か Go もキャメルケースが一般的って聞いた気がするんで!

Copy link
Contributor Author

Choose a reason for hiding this comment

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


export interface TaskListCardProps {
tasks: Task[];
status: 'todo' | 'doing' | 'done';

Choose a reason for hiding this comment

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

[IMO] めっちゃ小さいことなんやけど、
task とか todoリストとかって、 todo/not started , In progress, Done/Complete とかのイメージかも 🤔
そこまで違和感ないし人によるから、このままでも修正してもどっちでもいいと思う!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

僕もそう思ったので修正しました
[fix] statusをToDo/inProgress/Doneに修正

@imaimai17468
Copy link
Contributor Author

@YushiroDodo63
コンポーネントのtypeファイルについて、propsその他の型定義はtsxにかかず、別ファイルに書いた方が綺麗だと思ったので分離しています。CSS Modulesやビュー/ロジック分離と考え方は同じです。

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