Skip to content

ホーム・トークルーム一覧・トークルーム画面の作成(再)#13

Merged
1amageek merged 12 commits intostamp-team:masterfrom
thoth000:feature
Jun 16, 2020
Merged

ホーム・トークルーム一覧・トークルーム画面の作成(再)#13
1amageek merged 12 commits intostamp-team:masterfrom
thoth000:feature

Conversation

@thoth000
Copy link
Copy Markdown
Contributor

担当したissue

#5 #8

作成画面の見た目

(作成画面は自分が用意した中の最小デバイスでもレイアウトは問題なかったです)
device : Pixel 3 (Androidなので iOSと多少差はありますが、大きな違いはないと思います。)

Copy link
Copy Markdown
Contributor

@PictoMki PictoMki left a comment

Choose a reason for hiding this comment

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

先ほど電話で話した件コメントしました!
漏れてたらすみません・・。

Comment thread lib/talk_page.dart Outdated
Comment on lines +46 to +53
Widget room(
BuildContext context,
String name,
String image,
String lastMessage,
String time,
destination,
) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

�ここもStatelessWidgetがいいです!

Comment thread lib/room_page.dart Outdated
Comment on lines +113 to +119
Widget messageItem(
bool isMe, //IDとの一致で判定する予定
BuildContext context,
String message,
String time,
bool isRead,
) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ここもStatelessWidgetがいい!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

あといけそうだったらMesssgeモデルを作ってそれを渡すようにして欲しい!

Comment thread lib/home_page_tile_group.dart Outdated
Comment on lines +28 to +32
for (var info in infoList)
HomePageListTile(
info[0],
info[1],
info[2],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

model使わないとここの可読性が低くなってしまうので、
model作りましょう!
お時間ある時に少し説明します!

Comment thread lib/home_page.dart Outdated
Comment on lines +15 to +19
myGroups = [
[
"https://prtimes.jp/i/24101/70/resize/d24101-70-320114-0.jpg",
"Sport",
false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ここGroupモデルを作りましょう!

Comment thread lib/home_page.dart Outdated
Comment on lines +32 to +37
myFriends=[
[
"https://pbs.twimg.com/profile_images/581025665727655936/9CnwZZ6j.jpg",
"Alex",
false,
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ここユーザーモデル作りましょう!

Comment thread lib/main_page.dart Outdated
Comment on lines +67 to +68
Widget homeAppBar() {
return AppBar(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ここもStatelessがいい!

Comment thread lib/talk_page.dart Outdated
Comment on lines +9 to +16
room(
context,
"Sportssssssssssssssssssss",
"https://prtimes.jp/i/24101/70/resize/d24101-70-320114-0.jpg",
"hello,group!",
'23:04',
"",
),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Roomモデルを作って渡すようにして欲しい!

Comment thread lib/home_page_tile_group.dart Outdated
Comment on lines +28 to +33
for (var info in infoList)
HomePageListTile(
info[0],
info[1],
info[2],
),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

配列の時は配列.forEachにするとみやすい!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

早速すいません!
forEachのWidgetの返し方が分かりません!😢

Comment thread lib/home_page.dart Outdated
Comment on lines +16 to +20
[
"https://prtimes.jp/i/24101/70/resize/d24101-70-320114-0.jpg",
"Sport",
false,
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Groupモデルを作成してGroupを渡すようにして欲しい!

Comment thread lib/home_page.dart Outdated
Comment on lines +32 to +37
myFriends=[
[
"https://pbs.twimg.com/profile_images/581025665727655936/9CnwZZ6j.jpg",
"Alex",
false,
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Userモデルを作成して、Userを渡すようにして欲しい!

@1amageek
Copy link
Copy Markdown
Member

Model定義待ちですね。

Interfaceだけでも定義しておくと全然扱いが楽だと思います。

@PictoMki
Copy link
Copy Markdown
Contributor

レビューだと表現しきれない部分があったので、僕だったらこうするかなという実装をこちらにしています。
こちらdiffの確認用なので、マージはしないでください!

ソースにコメント残してるので確認お願いします!
https://github.com/thoth000/chat-flutter/pull/1

@thoth000
Copy link
Copy Markdown
Contributor Author

thoth000 commented Jun 15, 2020

@tomoki-inoue1221 へ
実装の方、見させていただきました!
ありがとうございます!😋

グループと友達一覧のふたつのリストだけなら、わざわざstatelessWidgetで用意する必要ないですか?🐲
あと、今言った一覧リストの上にContainer()がありますが、そこで指定されている高さはどういう意味をもってますか?🐮

@PictoMki
Copy link
Copy Markdown
Contributor

グループと友達一覧のふたつのリストだけなら、わざわざstatelessWidgetで用意する必要ないですか?🐲

どのStatekessWidgetのな話ですか?

コメントにも確か残していますが、特に意味はないです。

HomePage、TalkPage、RoomPageにそれぞれ実装。リスト表示はListView.builderを使った。
Comment thread lib/main_page.dart Outdated
Comment on lines +14 to +15
void changepage(int index) {
setState(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ここがキャメルケースになってないので、
changePageにしてください!

@PictoMki
Copy link
Copy Markdown
Contributor

flutter format .コマンドは実行してますか?
実行したらコード変わってしまうので、changePageを修正後再度実行してプッシュをお願いします!

@PictoMki
Copy link
Copy Markdown
Contributor

いい感じだと思います!

@1amageek
Copy link
Copy Markdown
Member

マージン、フォントサイズあたりがハードコードされてますね。

早めにどこかに定義した方が良さそうです。

@PictoMki
Copy link
Copy Markdown
Contributor

@1amageek
レビューありがとうございます!
ログイン・新規登録画面もハードコードされてるので、共通化処理を別のissueで切っているので
こちら一旦マージさせていただいてもよろしいですか?
#14

@1amageek 1amageek merged commit 9ca93d9 into stamp-team:master Jun 16, 2020
@thoth000 thoth000 deleted the feature branch June 17, 2020 06:50
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