Skip to content
This repository has been archived by the owner on Sep 4, 2024. It is now read-only.

Session/8 #21

Merged
merged 10 commits into from
Aug 26, 2024
Merged

Session/8 #21

merged 10 commits into from
Aug 26, 2024

Conversation

NaoyukiSugi
Copy link
Owner

@NaoyukiSugi NaoyukiSugi commented Aug 20, 2024

課題

close #9

対応箇所

  • Riverpod を導入して、天気予報画面の状態管理を見直す
  • アーキテクチャを見直し、ARCHITECTURE.md に記載する

動作

Screen_recording_20240820_194947.mp4

@NaoyukiSugi NaoyukiSugi self-assigned this Aug 20, 2024
@NaoyukiSugi NaoyukiSugi marked this pull request as ready for review August 20, 2024 11:23
@github-actions github-actions bot requested a review from nerd0geek1 August 20, 2024 11:23
Copy link

Ready for review 🚀

Base automatically changed from session/7 to main August 21, 2024 01:25
@nerd0geek1
Copy link
Collaborator

@NaoyukiSugi
[ask]
WeatherScreenで使用している各Widgetについてですが、子要素において、WidgetRef refを参照しないのであれば、
StatelessWidgetのままでも良いのではないでしょうか?

@nerd0geek1
Copy link
Collaborator

それとディレクトリ構成についても検討したほうが良いかもです!

@NaoyukiSugi
Copy link
Owner Author

@NaoyukiSugi [ask] WeatherScreenで使用している各Widgetについてですが、子要素において、WidgetRef refを参照しないのであれば、 StatelessWidgetのままでも良いのではないでしょうか?

確かに、ref参照しない部分はStatelessWidgetのままで良かったですね。
99b208f で修正しました。

@NaoyukiSugi
Copy link
Owner Author

それとディレクトリ構成についても検討したほうが良いかもです!

1e60533 でディレクトリ構成を修正しました。

あと、一部命名が不適切な箇所があったので 13cb74e で修正しました。

@nerd0geek1
Copy link
Collaborator

@NaoyukiSugi
大変、申し訳ない。
Slack通知が来ておらず、sugiさんの再レビュー依頼が漏れておりました。

まず、ディレクトリ構造については概ね良いと思いますが、mixin/についてはui/に移動して良いと思いました。
(mixin/自体はこのレイヤーにあって良いと思いますが、現在、mixin/内に配置されているファイルは、
StatefulWidgetを前提としているため)

Copy link
Collaborator

@nerd0geek1 nerd0geek1 left a comment

Choose a reason for hiding this comment

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

@NaoyukiSugi
先程、コメントさせていただいた点を除いて気になる点はありませんでした。

LGTM

@NaoyukiSugi
Copy link
Owner Author

@nerd0geek1
ありがとうございます。
d6d6247mixin/ui/ に移動しました。
Approve済み & 軽微な修正なのでそのままマージしちゃいます。

@NaoyukiSugi NaoyukiSugi merged commit d19fdb8 into main Aug 26, 2024
2 checks passed
@NaoyukiSugi NaoyukiSugi deleted the session/8 branch August 26, 2024 02:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Session8
2 participants