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

#71 仕事詳細ページの実装 #88

Merged
merged 5 commits into from
Jul 29, 2023
Merged

Conversation

RikitoNoto
Copy link
Collaborator

Issue

#71
close #71

説明

仕事詳細ページを実装しました。

  • 本来はjobIDをURLからパラメータとして取得しますが、現状ルーティングができていないため、jobIdは固定値で「PYRsrMSOApEgZ6lzMuUK」を使用しています。
  • Chipウィジェットを使用していますが、色をmain関数でテーマカラーに設定しています。
  • Enable、DisableのあるChipウィジェットは、アカウント画面でも使用するため、dart_flutter_commonに定義しています。
  • Figmaをみて、「ホストの業種」は対象外も表示していますが、「アクセス」は対象外は非表示にしていそうでしたので、合わせて作成していますが、認識が違いましたら修正します。

UI

a

その他

なし

チェックリスト

  • PR の冒頭に関連する Issue 番号を記載しましたか?
  • 本 PR の変更に関して、エディタや IDE で意図しない警告は増えていませんか?(lint 警告やタイポなど)
  • Issue の完了の定義は満たせていますか?

@RikitoNoto RikitoNoto added the 1. 機能開発 機能開発 label Jul 26, 2023
・JobDetailPage._buildSection => SimpleSection
・JobDetailPage._buildHostTypeChips, JobDetailPage._buildAccessTypeChips
  => RowChips
packages/dart_flutter_common/lib/src/row_chips.dart Outdated Show resolved Hide resolved
final Iterable<T> allData;

/// 選択肢をキーにしたデータのラベル
final Map<T, String> allLable;
Copy link
Owner

Choose a reason for hiding this comment

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

[タイポ] もしエディタにタイポチェックの機能が入っていなければぜひ入れることをおすすめします!

Copy link
Owner

Choose a reason for hiding this comment

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

あと、変数名は単に labels でもいいかも?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ご確認ありがとうございます!
修正しました。
タイポも入っていませんでしたので追加しました!

packages/dart_flutter_common/lib/src/row_chips.dart Outdated Show resolved Hide resolved
packages/dart_flutter_common/lib/src/row_chips.dart Outdated Show resolved Hide resolved
packages/dart_flutter_common/lib/src/row_chips.dart Outdated Show resolved Hide resolved
return Scaffold(
appBar: AppBar(
title: const Text('お手伝い募集'),
centerTitle: true,
Copy link
Owner

Choose a reason for hiding this comment

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

中央に配置するなら一括で MaterialApp の appBarTheme でやりたいので、ここは一旦消しましょうか!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

一旦削除しました!
とりあえずテーマは変更していませんが、今回のPRでテーマも変更しますか?
もしくは幾つか画面完成後にデザインをみて決定という形でしょうか?

Comment on lines 52 to 60
Widget _buildErrorPage(String message) {
return Center(
child: Text(message),
);
}

Widget _buildLoadingPage() {
return const Center(child: CircularProgressIndicator());
}
Copy link
Owner

Choose a reason for hiding this comment

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

この 2 つはいったん必要な箇所にべたっと書いてもらっていいです!(こういうエラーが起きた場合系の UI はあとで一括でうまくやる方法考えたいです!!うっすら考えておいてもらえたりするとめちゃめちゃ嬉しいです!!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正しました!

packages/mottai_flutter_app/lib/job/ui/job_detail.dart Outdated Show resolved Hide resolved
packages/mottai_flutter_app/lib/job/ui/job_detail.dart Outdated Show resolved Hide resolved
packages/mottai_flutter_app/lib/job/ui/job_detail.dart Outdated Show resolved Hide resolved
Copy link
Owner

@kosukesaigusa kosukesaigusa left a comment

Choose a reason for hiding this comment

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

ちょっとリファクタ追加して、Approve しました!!88eba3e

今回もとてもすばらしい実装・PR ありがとうございました!!

@RikitoNoto RikitoNoto merged commit 3e939de into main Jul 29, 2023
1 check passed
@RikitoNoto RikitoNoto deleted the 71_create_job_detail_view branch July 29, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. 機能開発 機能開発
Projects
None yet
Development

Successfully merging this pull request may close these issues.

仕事詳細ページの UI の実装
2 participants