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.

Sign up for GitHub

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

Jump to bottom

Session/11 #24

Merged
merged 21 commits into from
Oct 30, 2023
Merged

Session/11 #24

merged 21 commits into from
Oct 30, 2023

Conversation

mqkotoo
Copy link
Owner

@mqkotoo mqkotoo commented Oct 27, 2023 •
edited
Loading

課題

close #12

対応箇所

  • 使用する API を fetchWeather() から syncFetchWeather() に変更する
  • API の処理が戻るまで CircularProgressIndicator を表示する
  • 上記の変更に合わせてテストコードの修正をする
  • 上記の変更に合わせてARCHITECTURE.mdの修正をする。

動作

expected actual
expected
2023-10-27.14.40.53.mov

mqkotoo added 16 commits October 18, 2023 16:20
Change fetchWeather() -> syncFetchWeather()
1ae2593
Update WeatherStateNotifier based on changes of fetchWeather()
0d3f102
Wrap syncFetchWeather() with compute
9757bda
Create LoadingStateNotifier
4449e5a
Update generated files
1a8437a
Implement loading UI
b7c0aa0
Give key to CircularProgressIndicator widget
847982e
Update weather_service_test.dart based on changes of product files
25bc04b
Remove circularProgressIndicatorKey
8944c59
Update generated files based on session10 changes
309f4a3
Update weather_state_notifier_test.dart, weather_state_notifier.dart
e4142a9
Replace fetchWeather with syncFetchWeather
99581c4
Fix background color of CircularProgressIndicator
fc44afa
Move loading state notifier to state/, update how to handle loading view
2ce2369
Replace json dummy data with data of Result type.
44c86ec
Update weather_page_test.dart: mock WeatherService, add loading test
d0d2619
mqkotoo
mqkotoo commented Oct 27, 2023
View reviewed changes
lib/service/weather_service.dart Show resolved Hide resolved
test/view/weather_page_test.dart Show resolved Hide resolved
test/view/weather_page_test.dart Show resolved Hide resolved
@mqkotoo mqkotoo marked this pull request as ready for review October 27, 2023 06:31
@github-actions
Copy link

Ready for review 🚀

@mqkotoo mqkotoo changed the title Session/11 [WIP]Session/11 Oct 27, 2023
@mqkotoo mqkotoo changed the title [WIP]Session/11 Session/11 Oct 27, 2023
Yamasaki-pan961
Yamasaki-pan961 approved these changes Oct 30, 2023
View reviewed changes
Copy link
Collaborator

@Yamasaki-pan961 Yamasaki-pan961 left a comment

Choose a reason for hiding this comment

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

1点だけ指摘していますがApproveしちゃおうと思います！
研修お疲れ様でした🥳
LGTM

test/view/weather_page_test.dart Show resolved Hide resolved
test/state/weather_state_notifier_test.dart Show resolved Hide resolved
lib/view/weather_view/weather_page.dart Show resolved Hide resolved
lib/state/weather_state_notifier.dart Outdated Show resolved Hide resolved
@mqkotoo mqkotoo merged commit b6380d7 into main Oct 30, 2023
3 checks passed
@mqkotoo mqkotoo deleted the session/11 branch October 30, 2023 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yamasaki-pan961 Yamasaki-pan961 Yamasaki-pan961 approved these changes

@morikann morikann Awaiting requested review from morikann

@trm11tkr trm11tkr Awaiting requested review from trm11tkr

Assignees

@mqkotoo mqkotoo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Session11
2 participants
@mqkotoo @Yamasaki-pan961