模擬案件_#7_addErrorPage

yasyun commented 2 years ago

issue：#7
レビューして欲しい点新しく追加したNetworkErrorView.swift, NetworkErrorView.xib, NotLoginPageViewController.swiftを中心的に見ていただきたいです。また、FeedPage, TagListPage, MyPageのコードと、CommonApi.swiftにエラーに関する記述を加えたので、その点もチェックをお願いしたいです。
その他・issueにも記述してありますが、変更点が思っていたよりも多くなってしまったため、いったんFeedPage, TagPage, MyPageの３画面のみでPull Requestを提出し、LGTMをいただいてからOAuthPage, FollowPage, FeedDetailPage, TagDetailPage, ArticlePageのErrorViewを実装してレビューをいただく形にしようかと思っています。その際はブランチを切った方がよいでしょうか？・各ページのAPIのリクエスト時にエラーが起きた場合、空の配列を返すことによって、TableViewにデータを詰め込む際に空の判定をしてErrorPageを表示しています。(FeedPageで検索して配列の中身が空だった場合は検索結果無し画面を表示)

ShuheiYoshidaJP commented 2 years ago

タグ画面にレイアウトのぐずれが出るようです。検証端末：iPhoneX（実機）

あと現状ではwifiとモバイルデータ通信を切っての検証でしかエラー画面が現れないです。 network-conditionerを使用すると通信量を制限して検証できるので試してみてください。

kaitoさんにLGTMもらったらレビューみたいと思います。

Kaito-a-bit commented 2 years ago

NotLoginPageViewControllerについてですが、現状の画面遷移のままだとViewがスタックしてしまうので、他の手法を考えましょう。

Kaito-a-bit commented 2 years ago

・issueにも記述してありますが、変更点が思っていたよりも多くなってしまったため、いったんFeedPage, TagPage, MyPageの３画面のみでPull Requestを提出し、LGTMをいただいてからOAuthPage, FollowPage, FeedDetailPage, TagDetailPage, ArticlePageのErrorViewを実装してレビューをいただく形にしようかと思っています。その際はブランチを切った方がよいでしょうか？

現状かなり差分が大きくてレビュー漏れが発生してしまっているので、もっと細かくPRもしくはブランチを分けてください。レビュー漏れを防ぎたいので以下のことに気をつけてもらえると助かります。・コミットの頻度を細かくする　→コミット毎の差分が大きい為、レビューするべき点が定まらないです。　　例えば今回であれば、FeedPage, TagPage, MyPageへのエラー処理関連で3つのコミットに分けることができます。

・1つのプルリクエストで実装する機能単位を小さくする　→もう少しPRで実装する機能を限定した方が良いかと思われます。　　例えば今回であれば「エラー画面の実装」と「非ログイン画面の実装」でPRを分けるなど、です。

Kaito-a-bit commented 2 years ago

後、このPRでの作業が終わったらファイル整理をお願いします。

yasyun commented 2 years ago

2021-11-17.0.29.17.mov

エラーが起こっている状態にも関わらず、リロードボタンで元いた画面に戻ってしまいます。

dataが取得できていた場合にのみ、NetworkErrorViewをremoveFromSuoerView()するように変更しました。

yasyun commented 2 years ago

NotLoginPageViewControllerについてですが、現状の画面遷移のままだとViewがスタックしてしまうので、他の手法を考えましょう。

今まではContainer Viewを使用してNotLoginPageViewControllerを表示していましたが、今回kaitoさんと同じくコードでUIViewをaddSubViewして表示する形に変更しました。これによって今までisHidden()だったものをremoveFromSuperView()に変更することができたので、おそらくスタックするという現象は改善できたのではないかと思います。また、NetworkErrorViewもStoryboard上にUIVIewを置いて表示させていましたが、こちらも上記と同じ仕様に変更しました。なお、差分が大きくなることを防ぐため、TagListPageViewControllerに関してはまだそのままになっています。この仕様でOKをいただいてから、このページも同じように変更しようと思います。

Kaito-a-bit commented 2 years ago

ビルドした所、エラーが出ました。

再現手順(iPhone13) ・ログインする・タグページを押す

yasyun commented 2 years ago

ビルドした所、エラーが出ました。

再現手順(iPhone13) ・ログインする・タグページを押す

なぜかStoryboard上のUIViewのCustomClassが外れてしまっていたので、設定したところ動作しました。確認不足ですみませんでした。

Kaito-a-bit commented 2 years ago

TagListPageViewControllerにのみ、エラー処理が確認できませんでした。yasyunさんの方でエラー処理が同画面で動作するか確認してほしいです。そのほかの点についてはOKです。

yasyun commented 2 years ago

TagListPageViewControllerにのみ、エラー処理が確認できませんでした。yasyunさんの方でエラー処理が同画面で動作するか確認してほしいです。そのほかの点についてはOKです。

こちらのPullRequestにもコメントしたように、TagListPageについてはFeedPageとMyPageの実装でOKをもらってから、同じ様に実装するという話だったので、先ほどTagListPageの方にも全く同じ様に実装しました。

Kaito-a-bit commented 2 years ago

OKです。このようなことが起きてしまうのでPRは分けていくようにお願いします👍

ShuheiYoshidaJP commented 2 years ago

AppDelegate.swift:12

Thread 1: "Attempted to scroll the table view to an out-of-bounds row (0) when there are only 0 rows in section 0. Table view: <UITableView: 0x7fc812092e00; frame = (20 191; 388 652); clipsToBounds = YES; autoresize = RM+BM; gestureRecognizers = <NSArray: 0x6000015f49f0>; layer = <CALayer: 0x600001a35720>; contentOffset: {0, 0}; contentSize: {388, 0}; adjustedContentInset: {0, 0, 0, 0}; dataSource: <Qiita_App.FeedPageViewController: 0x7fc812a76f50>>"

このようなエラーが発生しました。 ↓再現方法

wifiを切る
feedページ読み込み失敗し、エラー画面が表示される
再読み込みボタンをタップする

再現できないようでしたら、ご連絡ください。

またnetwork conditionerを使用するとエラー画面が表示されないです。

network conditioner対応していただけると助かります。

yasyun commented 2 years ago

network conditioner起動時にアプリを起動して、リクエストがタイムアウトしたときにエラー画面が表示される形に修正しました。

yasyun commented 2 years ago

AppDelegate.swift:12
Thread 1: "Attempted to scroll the table view to an out-of-bounds row (0) when there are only 0 rows in section 0. Table view: <UITableView: 0x7fc812092e00; frame = (20 191; 388 652); clipsToBounds = YES; autoresize = RM+BM; gestureRecognizers = <NSArray: 0x6000015f49f0>; layer = <CALayer: 0x600001a35720>; contentOffset: {0, 0}; contentSize: {388, 0}; adjustedContentInset: {0, 0, 0, 0}; dataSource: <Qiita_App.FeedPageViewController: 0x7fc812a76f50>>"
このようなエラーが発生しました。 ↓再現方法

wifiを切る

feedページ読み込み失敗し、エラー画面が表示される

再読み込みボタンをタップする

再現できないようでしたら、ご連絡ください。

またnetwork conditionerを使用するとエラー画面が表示されないです。

network conditioner対応していただけると助かります。

こちらで何度か試してみてもうまく再現できなかったので、もう少し詳しい説明くださると非常に助かります。

ShuheiYoshidaJP commented 2 years ago

demo 動作確認できました。LGTMです。

shinonome-inc / mobile_sakai

模擬案件_#7_addErrorPage #36