search

shinonome-inc / mobile_shu_mogi_Qclient

0 stars 1 forks source link

pull to refreshしたら一回cellが消えてしまう #34

Closed ShuheiYoshidaJP closed 3 years ago

ShuheiYoshidaJP commented 3 years ago

作業ブランチ

feature/#34fix_reload_tableView_logic

現在のデータ取得→tableViewリロードのロジック

dataItemsが更新されるたびにリロード

目標のデータ取得→tableViewリロードロジック

一回のデータ取得で得られたデータが全てdataItemsに格納されたら tableViewをリロードする

他の修正ポイント

プライバシーポリシー・利用規約ページにおいてメールアドレスの削除・編集を無効にする 2回目以降のログインを飛ばす機能を復活させる

ShuheiYoshidaJP commented 3 years ago

プライバシーポリシー・利用規約画面に編集を加えた

textViewを編集不可に設定 メールアドレスを削除 bottomを一致（safeArea→superView）

ShuheiYoshidaJP commented 3 years ago

2回目のログインだとログインをスキップさせる機能を復活させた

修正箇所

LoginVCのviewDidLoadメソッド内

ShuheiYoshidaJP commented 3 years ago

ErrorViewクラスでviewControllerのTabBarControllerが取得できていないことが判明した

今までErrorViewクラスのcheckSafeAreaメソッドの引数でViewControllerを引き渡し、 メソッド内でViewControllerのnavBarとTabBarを取得していたが、 今までの修正の影響でtabBarがメソッド内で取得できないことが判明した。 原因は不明。

ShuheiYoshidaJP commented 3 years ago

https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/issues/34#issuecomment-787400984 の原因が特定でき、解決できた

原因

問題が起っているのはArticlePageのみ ErrorView作成当時はArticlePageがPush遷移だった ErrorView作成後、ArticlePageをModal遷移に変更したため ArticlePageにはTabBarが存在しない仕様になった

ErrorViewのcheckSafeAreaメソッドは navControllerとtabControllerのどちらかがnilだと機能しないようにしている よってArticlePageにおいてtabbarがnilだったため 今回の不具合がおきた

解決方法

tabBarControllerがnilだった場合、tabBarHeightを0にする

修正ポイント

ErrorViewクラスにて

func checkSafeArea(viewController: UIViewController) {
        let tabBarController = viewController.tabBarController
        guard let navBarController = viewController.navigationController else { return }
        guard let tabBarHeight = tabBarController != nil ? tabBarController?.tabBar.frame.height : 0 else { return }
        let navBarHeight = navBarController.navigationBar.frame.height
        let height = viewController.view.frame.height - tabBarHeight - navBarHeight
        frame = CGRect(x: 0.0,
                       y: navBarHeight,
                       width: viewController.view.frame.width,
                       height: height)
    }
ShuheiYoshidaJP commented 3 years ago

現状では以下のようにdataItemsを定義しているため dataItemsに変更があれば、tableViewのリロードが行われる https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/blob/90c4faf65b546f68de3ed998c1195c1fbd066031/Qcli/ViewController/FeedViewController.swift#L15-L19

これをデータ取得が終わってからリロードさせたいため 以下のコードに書き換えた

var dataItems = [ArticleData]()

現状でのデータ取得メソッドは以下のコードで行っている https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/blob/90c4faf65b546f68de3ed998c1195c1fbd066031/Qcli/ViewController/FeedViewController.swift#L78-L105 このコードでの問題点は 仮に93行目と94行目の間にtableViewのリロード処理をはさむと 以下の問題が発生する https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/issues/21#issuecomment-769615998 ↑dataItems更新の前にtableViewのリロードが行われてしまう

したがって dataItemsの更新終了後にtableViewの更新を行いたいため 以下のメソッドを用意した

    func storingData(dataArray: [AirticleModel], completion: () -> Void) {
        dataArray.forEach { (oneAirticleData) in
            if let title = oneAirticleData.title,
               let createdAt = oneAirticleData.createdAt,
               let like = oneAirticleData.likesCount,
               let imageURL = oneAirticleData.user.profileImageUrl,
               let articleURL = oneAirticleData.url,
               let id = oneAirticleData.user.id {
                let oneData = ArticleData(id: id, imgURL: imageURL, titleText: title, createdAt: createdAt, likeNumber: like, articleURL: articleURL)
                self.dataItems.append(oneData)
            } else {
                print("ERROR: This data ↓ allocation failed.")
                print(oneAirticleData)
            }
        }
        completion()
    }

またデータ取得のメソッドを以下のように書き換えた

    func getData(requestAirticleData: AirticleDataNetworkService) {
        requestAirticleData.fetch(success: { (dataArray) in
            guard let dataArray = dataArray else { return }
            self.storingData(dataArray: dataArray,
                             completion: {
                                self.articleTableView.reloadData()
                                print("👍 Reload the article data")
                                self.isNotLoading = true
                             })
        }, failure: { error in
            print("Failed to get the article list data.")
            if let error = error {
                print(error)
            }
            self.isNotLoading = true
            //TODO: エラー画面を作成し、遷移させる
        })
    }

クロージャを使ってdataItemsを更新してから tableViewのリロードを行うようにした

ShuheiYoshidaJP commented 3 years ago

https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/issues/34#issuecomment-787601900 こちらの修正の後に正常にデータ取得して、tableViewのUI更新ができるかを検証した 下スクロールのデータ取得・UI更新は成功した pull to refreshをすると以下のエラーが表示された 2021-03-01 14 42 27 Fatal error: Index out of range

原因調査

デバッグをとるために以下のような出力を tableView -> UITableViewCell メソッド内に配置した

print("test: \(indexPath.row)/\(dataItems.count)")
    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        guard let cell = articleTableView.dequeueReusableCell(withIdentifier: "ArticleCell", for: indexPath) as? ArticleTableViewCell else {
            abort()
        }
        print("test: \(indexPath.row)/\(dataItems.count)")
        let model = dataItems[indexPath.row]
        cell.setModel(model: model)
        return cell
    }

再度pull to refreshの検証を行った

test: 0/20
test: 1/20
test: 2/20
test: 3/20
test: 4/20
test: 5/20
test: 6/20
test: 7/20
test: 8/20
test: 9/20
test: 10/20
test: 11/20
test: 5/0

最後の

test: 5/0

が表示された時にエラーが起こったことがわかった

なぜdataItemsのcountが0になってしまうのか

pull to refreshが呼び出してくるメソッド内に原因があるのではないかと思った https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/blob/90c4faf65b546f68de3ed998c1195c1fbd066031/Qcli/ViewController/FeedViewController.swift#L114-L121 このメソッド内の

dataItems.removeAll()

が処理されて、dataItemsのcountが0になってしまうことがわかった

ShuheiYoshidaJP commented 3 years ago

https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/issues/34#issuecomment-787669714 の対処法（案）

下記のメソッドの https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/blob/90c4faf65b546f68de3ed998c1195c1fbd066031/Qcli/ViewController/FeedViewController.swift#L114-L121

dataItems.removeAll()

を削除する これを行うことで新しく得たデータはtableViewの最下のCellから反映される getDataメソッドが実行される前に あらかじめ

let beforeFetchDataCount = dataItems.count

でTableViewに表示されているデータ数を取得しておく そしてデータ取得後に上からbeforeFetchDataCountの分だけ dataItemsのデータを削除して tableViewをリロードさせる。

ShuheiYoshidaJP commented 3 years ago

対処方法を少々変更した

以前考えていた対処方法同様 refreshメソッドにおいて https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/blob/90c4faf65b546f68de3ed998c1195c1fbd066031/Qcli/ViewController/FeedViewController.swift#L114-L121 から

dataItems.remove()

を削除する

pull to refresh 時 getDataメソッド内で既存データを削除する処理を行う

getDataメソッドの引数にisRefreshのブール値を用意する（デフォルトなfalse） 全てのデータが取得し終わった時に isRefreshがtrueの場合に前回の取得データを削除して 新しく取得したデータだけを残す https://github.com/shinonome-inc/mobile_shu_mogi_Qclient/blob/e2a951a1b3eb0bfdd63b7c75cbc2d526406e15ca/Qcli/ViewController/FeedViewController.swift#L74-L95

この処理によって dataItemsが空になることを避けることができ tableViewに何も表示される状態がないようにすることができた