Ayato-kosaka / spelieve

1 stars 0 forks source link

【BUG】URLでしおり画面に入ると他のしおりを選択できなくなる #960

Open Ayato-kosaka opened 7 months ago

Ayato-kosaka commented 7 months ago

Bug report

Describe the bug

A clear and concise description of what the bug is. URLでしおり画面に入り、HelloSpelieve に戻った後、他のしおりを選択すると、URLで入った画面が写ってしまう。

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://www.spelieve.com/ItineraryEdit?itineraryID=eTUHLh152sFWMNGLLwSg
  2. hello Spelieveに戻る
  3. しおりBをクリック
  4. しおりAの内容が表示される

Expected behavior

A clear and concise description of what you expected to happen. しおりBが映るべき。

Screenshots

If applicable, add screenshots to help explain your problem.

Environment

Additional context

Add any other context about the problem here.


Solution result

Cause (バグの原因)

ナビゲーションの考慮ミス。 しおりAにURLに入った時点で、Stackが生成され、その後 HelloSpelieve に遷移しても、HelloSpelieve の Stack は作られていないため、Stack A は残る。 その後、しおりB に navigation.navigate すると、 Stack A があるため、新規作成されない。

How to fix (どのようにして解決したか)

■ 課題① 単票画面のStack管理 問題点: URL経由で単票画面に直接アクセスすると、一覧画面へ戻った際に、予期せぬ単票画面のStackが残留することが分かった。 特定のシナリオでは、Stackの残留が問題を引き起こす可能性がある。 例えば、新規画面への再エントリ時にInitialize処理を実行する必要がある場合などです。 そのため、Stack を常に新規で積む方法を検討する必要がある。

<対策> 案1 ☆ この案を採用する 画面遷移には基本 push を採用し、navigate は、Screen を再利用したい場合に利用する。 デメリット:BottomTab を跨いだ navigation.push が不可能であるため、異なるタブ間での新規作成時には、目的のタブに遷移せず、現在のタブ内でStackを積む形になります。 → 課題②で対応する。

単票画面パターンにおけるDB取得処理概要を比較検討 新規モード 更新モード Subscribe 無 Subscribe 有 遷移前 遷移後 処理内容
    undefined undefined setState(initalModel)
    value undefined 同上
    undefined undefined 1. firestore#addDoc2. onSnapshot(React#setDocSnap)
    value undefined 同上
    undefined value setState(Firestore::getDoc)
    value1 value2 同上
    undefined value onSnapshot(React#setDocSnap)
    value1 value2 同上

案2 遷移前の履歴クリア: 一覧画面へ戻る際に履歴をクリアする実装を採用する。 メリット:非初期画面からエントリーした際に、不要なStackを削除できます。 →navigate を確り使い分けることで、navigate を使う = 積まれている Stack に戻りたいという意味になるので、不要なStackが残っても問題なくなる。 デメリット: 例えば、顧客検索→顧客A→顧客Aの口座検索→口座A の動線で画面遷移した際に、口座検索で clear history が実装されることになり、前画面に戻れなくなり、アプリの利便性が非常に悪くなる。

■ 課題② 別サービスの画面に navigation.push で遷移出来ない 問題点: 現状の設計コンセプトは、Bottom Tab 毎にそのサービスに属すページのみを管理している。そのため、別サービス画面に遷移する際に BottomTab を跨いぐこととなり、 navigation.push が不可能である。 対策: 全てのページをナビゲーションするコンポーネントを作成し、それを 全ての BottomTab から呼び出す。

■ 課題③ Web遷移の考慮漏れ 問題点: navigationEvent#tabPress では、最初の Stack に積まれている画面に遷移するが、 web の場合 URL から奥画面に直接遷移することがあり、初期画面に遷移するわけではない。

初期画面に遷移する手段がない課題に対する対策: タブボタンから初期画面遷移を可能にする。 想定外の挙動が発生するリスク: 課題①のケースは検討済み。それ以外は起きてみないとわからない。

■ 課題④ createItineraryの初期値設定 問題点: createItinerary関数への初期値渡し方が最適ではありません。全ての初期値を画面遷移パラメータとして渡す方法は、保守性に欠けます。

対策: この問題は今回の改修で影響しないため、一時保留とします。

bk bk_画面毎にPush の要否整理

Roll Out

他機能への影響範囲と対応計画を記載する https://docs.google.com/spreadsheets/d/1k3ZWrhZxykUTT2BVzJVIQZ7KabOm70SGZwN81n_NlQA/edit#gid=0

Ayato-kosaka commented 7 months ago

884 の対応漏れ?

いや、Navigation の動きの考慮漏れかな、、 ①ItineraryEdit に URL 遷移する。 → Root Stack -> BottomTab -> ItineraryStack -> ItineraryTopTab -> ItineraryEdit を初期配置 ②HelloSpelieve に遷移する → ItineraryStack に HelloSpelieve が積まれる ③別のしおりを選択する → ①のStackに戻る

の動きをしているのかも? 想定していた動き → しおりの選択では、新しいStackが積まれる想定だけど、基盤が ItineraryEdit だから挙動が違うのか。。

挙動の理由 navigation.navigate は、既に積まれてたらそこに戻るが積まれていなければ新規作成する。 Hello Spelieve にエントリーする場合、 ItineraryEdit は、navigation.navigate で積まれて、goBack で削除されてを繰り返すから、navigation.navigate 押下時は常に新規で積む。 ItineraryEdit にエントリーする場合は、Hello Spelieve への遷移で、ItineraryEdit のStack が削除されないから、navigation.navigate 押下時は詰めれている所に移動する。

解決策 HelloSpelieve からの遷移で router push すれば良いのではないか。

Ayato-kosaka commented 6 months ago

IPA003EditPlan に URL で入ると元画面に戻れないバグを一緒に対応したいなぁ〜 けど、まぁええか

Ayato-kosaka commented 6 months ago

検討メモ ・PAをまとめる Stack はBottom Tab 単位で共有させる? ・性能的にも、Stack積まれるまでレンダリングさせないから問題ない?

具体的には ITopTab PPA1 PPA2 TPA1 TPA2 … をコンポ化する

Root Tab  構成: Bottom Navigator + モーダル※ページコンポ(初期はなんでも) 初期: Bottom Bottom tab 構成: Itinerary※ページコンポ継承 + Place※ページコンポ継承 初期: Itinerary

Ayato-kosaka commented 5 months ago

navigation.navigate の挙動についての課題

  1. ItineraryEdit?itineraryID=XXX に URL から遷移
  2. タブから HelloSpelieve に遷移する。
  3. ItineraryEdit?itineraryID=YYY に遷移する
  4. タブを押下する。 の点順を踏むと、HelloSpelieve に遷移せずに、ItineraryEdit?itineraryID=XXX に遷移する。

→ 他ボタンで試したがタブ固有の仕様っぽい

navigation.addListener('tabPress') の処理を外だししていたのが原因だったみたい。なぜ不具合が起きたのかは不明なため、一旦切り出しをやめる。

Ayato-kosaka commented 5 months ago

対策: 全てのページをナビゲーションするコンポーネントを作成し、それを 全ての BottomTab から呼び出す。

現状のルート、あるべきルートを整理する。

■ 統合コンポーネントは、StackGroup or StackNavigator? ・Root Stack.Navigator から遷移する場合、StackNavigator を利用すると Navigator が二重になる。 ・StackNavigator の initialRouteName を props 渡しするのは面倒。 ・Navigator 配下には、コンポーネントを配置できないため難しいかも →済

screenOptions={{ presentation: 'modal' }}って何? ・https://reactnavigation.org/docs/modal/ → 廃止する。EditPlan は、Thumbnail と同階層に。

■ navigationInterface 見直したい →これ何するの?

■ routeName とか文字列やめたい ・Interface で守ってるからまだましではある。 ・機能ID振っているものはクラス定数で定義したい。 ・機能ID振っていないものは再整理。(機能割り振りしたいかも)

■ 画面遷移の無影響確認は必須(code-analysis で) ・Bottom Tab の自画面遷移が壊れる ・Bottom Tab を超えて遷移していたものが超えなくなるかも  → PlanEdit から戻れない

■ URL現新不一致 http://localhost:19006/ItineraryEdit?itineraryID=XEef0tlZJ9muHXk6d6We

Ayato-kosaka commented 5 months ago

TODO ・課題2対応 → プロトタイプを反映する ・課題1のUT(スプレッドシート)  ・画面遷移全検証  ・ボトムタブ押下検証  ・画面スクリーン現新検証

補足 ・課題3のUTはノンエビデンスである程度見れたのでOK