コンポーネントの統廃合

[y-chan]

改善詳細 / Details of Improvement

• カードによって日付表示形式の揺れがある #4995 のように、表記にブレが出るなどメンテナンスが行いにくくなっている。
• さまざまな変化ののちに、いずれかのコンポーネントと同じような機能を提供するコンポーネントが増え始めている。
• 整理として、同じような機能を提供しているコンポーネントを比較的昔からあるものに統合し、差分はpropsなどで制御。使用していたコンポーネントを廃止(削除)すべきと思われるが、propsや条件分岐を増やしすぎるとあまりよろしくないというのも聞くので要検討。
• 加えて、複数個所で使うことになっているコンポーネントに関しては名前を汎用的にすることも検討すべきと思われる。

[y-chan]

論点としては同じコードが複数ある状態を許容して現在のまま行く(こちらはメンテナンスの面で問題がある)のか、propsや条件分岐を増やしてコードをまとめる(こちらは実行速度の差が出る、コードが複雑化する)のかだと思います。
メンテナーだけでなく、コントリビューターの皆様のご意見が聞きたい所存です。

現状まとめられるものは複数見受けられますが、すべてを探し切れていないので、後で挙げておきます。

[goki90210]

それぞれのコンポーネントに継承関係があればよいのですが…。

#4176 の件でDataViewからいくつか辿ろうとしましたが途中でわけがわからなくなり断念してしまいました。

[y-chan]

今回の場合、主にグラフにおいての話題となります。そのほかのコンポーネントに関しても、統廃合できるのであればやるとよいのではないでしょうか(ただ、影響範囲がとてつもなく大きくなりそうな気はします....)
グラフですと、formatDayBeforeRatioメソッドで言えば同じコードが各コンポーネントに点在していますし、あまりよろしくないと思います...
displayInfoの差なんかは、propsで渡せるようにして、component/cards下のコンポーネントで作成して渡すようにしてもよいでしょう。
また、日別と累計の切り替えボタンを表示するか否かでもコンポーネントが増えているので、こちらもpropsで制御できるとよいのではないでしょうか。

[shgtkshruch]

メンテナンス性を考慮して、同じような役割のグラフやコードを整理するのは賛成です。
役割とデータ型が共通していて、props や継承などの縦の関係でまとめられるところは、無理のない範囲で共通化できると良いと思います。

一方で、縦の関係で共通化しにくいところは、横の関係でコンポーネントを作っていくと良いかなと思っています。
例えば、data-view-table やグラフの横スクロール、前日比の計算ロジックのように、グラフをまたいで同じ処理をしている部分です。
このサイトは「モニタリング指標」のように一気にグラフが入れ替ることがあるので、コンポーネントを組み合わせればグラフを実装できるようにしておくこと（柔軟性）も重要な指標かと思います。

個人的には、横の関係でコンポーネント化を進めつつ、できるところは縦の関係でまとめていけると良いかなと思っています。
縦の関係でまとめるとグラフのパターンが整理されますし、横の関係が整理されると今後グラフのバリエーションが増えても対応しやすくなると思います。
もちろん、作業的にやりやすい場合やすでにパターンが見えているところは縦の関係から整理してもいいと思います。

横の関係で共通化できそうだと思っているところは、以下のようなところです。

• GraphLegend のテンプレートとスタイル部分
• displayOption の共通部分
• displayOptionHeader は それぞれのグラフの displayOption から自動生成できそう
• mounted で aria-labelledby を付けているところ

[y-chan]

コンポーネントを組み合わせればグラフを実装できるようにしておくこと（柔軟性）

この発想はなかったです...でもとても良いと思います。

計算ロジック等のVueファイル内でなくても処理できるものに関してはutils下のtsファイルにまとめることを考えています。それ以外はコンポーネントでまとめるのが良さそうです。

私自身まだまだVueを触り始めたばかりなので継承を使うのもありだということに気づきませんでしたが、これも良さそうですね。

私だけでは思い浮かばなかった統廃合案が出来そうです...!

Edited: #5077 で日付計算関連がすでにまとめられていて、こんな感じを予想していました...!

[y-chan]

では、少々長いですが統廃合案を挙げていきます。
日付の表記の違いなどありますが、 #5077 であらかた解決されそうなので含んでいません。

廃止案

1. MonitoringConfirmedCasesChart.vue <モニタリング項目(1)新規陽性者数>
   1. 統合先: MixedBarAndLineChart.vue
   2. 差分:
      1. グラフの色
         • propsで切り替えられるようにする。
      2. デフォルトの数字フォーマッタ(小数点以下の処理)
         • そもそもpropsで渡せるようになっているので、都合のいいフォーマッタをデフォルトにする。
      3. tableLabelspropsの有無
         • required: falseにし、渡されている場合はそのまま使い、渡されていない場合はdataLabelsを使うようにする。
      4. データがnullの際の処理の有無
         • nullの場合にグラフが描画されなくなる問題が起きることがあるので、MixedBarAndLineChart.vueに合わせてnullの際の処理を入れるようにする。
      5. その他propsのデフォルト値の違い
         • MixedBarAndLineChart.vueに合わせる。
   3. 他のグラフと比べた際の差分
      1. グラフ内tooltipの日付がnuxt-i18nによるものではない(MonitoringConfirmedCasesChartもそうですが、統合先のMixedBarAndLineChart自体が$dで出力されたものを使っていないので挙げています。)
2. SevereCaseBarChart.vue <モニタリング項目(7)重症患者数>
   1. 統合先: TimeBarChart.vue
   2. 差分:
      1. transition、cumulativeの有無
         • 日別/累計などの処理。tableHeadersやtableData、切り替えボタンを描画するか否かに影響する。TimeBarChart自体を編集し、propsで制御できるようにするのがよさそう。
      2. 2段タイトル
         • グラフタイトルが長いことから、2段に分けるIssue/PRにより発生。1段しか使わない場合でも共通したコードで書けそうなので、TimeBarChart自体を2段タイトルに対応する形になりそう。
      3. displayInfo関数内の差
         • 返されるinfoデータが違う。現在のTimeBarChartのものはdefaultとしておき、components/cards/下で生成し、propsとして渡せるようにすることで対応できそう。
3. MonitoringConsultationDeskReportChart.vue <受診相談窓口における相談件数>
   1. 統合先: MixedBarAndLineChart.vue
   2. 差分:
      1. 2段タイトル
         • SevereCaseBarChart.vue <モニタリング項目(7)重症患者数>の差分2と同じ
      2. デフォルトの数字フォーマッタ(小数点以下の処理)の有無
         • MixedBarAndLineChart.vueに合わせ、フォーマッタを設定する
      3. tableLabelspropsの有無
         • MonitoringConfirmedCasesChart.vue <モニタリング項目(1)新規陽性者数>の差分3と同じ
      4. データがnullの際の処理の有無
         • MonitoringConfirmedCasesChart.vue <モニタリング項目(1)新規陽性者数>の差分4と同じ
      5. その他propsのデフォルト値の違い
4. ConfirmedCasesIncreaseRatioByWeekChart.vue
   • そもそも使われていないのでそのまま削除？

継承案

1. DashedRectangleTimeBarChart.vue <モニタリング項目(6)入院患者数>
   1. 継承元: TimeBarChart.vue
   2. 差分:
      1. 点線枠の描画
         • もともとこのコンポーネント自体私が作ったが、破線矩形の描画をしなくても使えるようしようとしていたので、TimeBarChartに統合するのもありだがいろいろ処理が変わっていることから継承が良いと判断。
      2. transition、cumulativeの有無
         • SevereCaseBarChart.vue <モニタリング項目(7)重症患者数>の差分1と同じ
      3. 2段タイトル
         • SevereCaseBarChart.vue <モニタリング項目(7)重症患者数>の差分2と同じ

共通化案

※ コンポーネント単位ではなく、関数/スタイル単位です

1. GraphLegend
   1. 影響範囲
      • 横スクロールを適用しているグラフで、見るグラフを選択できるもの
        • onClickLegend関数
        • スタイル
        • template内の<ul></ul>で囲まれている部分
        • data()内のdisplayLegendsやcolorsも?
   2. 共通化方法
      • Vueコンポーネント化
      • 模様などはpropsで制御できるようにする。
2. displayOption
   1. 影響範囲
      • 全グラフ?
   2. 共通化方法
      • utils/下にgraphUtils.tsのようなファイルをつくり、必要な情報を投げれば自動で生成される関数を作る。
3. displayOptionHeader
   1. 影響範囲
      • 横スクロールを有効化しているグラフ
   2. 共通化方法
      • utils/下にgraphUtils.tsのようなファイルをつくり、displayOptionを投げれば自動で生成される関数を作る。
4. displayDataHeader
   1. 影響範囲
      • 横スクロールを有効化しているグラフ
   2. 共通化方法
      • utils/下にgraphUtils.tsのようなファイルをつくり、displayDateに応じて適当なデータを生成する関数を作る。

UntrackedRateMIxedChart.vue<モニタリング項目(3)新規陽性者における接触歴等不明者数/増加比>や、PositiveRateMixedChart.vue<モニタリング項目(4)検査の陽性率/検査人数>も統合できればいいんでしょうけど、個人的にはどう統合していいかがわからず....

[nard-tech]

#5276 からこちらに誘導されたので参りました．

コンポーネントの統廃合は私も賛成するのですが，それ以前に，ディレクトリ構成が荒れていていることに問題意識をもっております．
ディレクトリ構成を見直し，コンポーネント間の依存関係を把握しやすくしてからコンポーネントの統廃合できそうな箇所を洗い出すのでもよいかなと思います．
なお，ディレクトリ構成の見直しについては #2790 (comment) でコメントした後，2度 PR を作成しました (#2916, #4016) が， conflict が多発したためいずれも close したという経緯があります．
とりあえず draft で PR を作ってみますので，ご参考いただければと思います．

[y-chan]

@nard-tech 以前よりディレクトリの整理が必要という意見はお見かけしておりました...!
確かにそうですね、グラフの追加や廃止が現状収まっているので、そちらを優先した方がいいかもしれませんね。

[nard-tech]

@y-chan ありがとうございます．作業中ですが，#5327 を作成しました．

[kaizumaki]

@y-chan 長らくお待たせしてしまいました。componentsのディレクトリ構成の見直しがなされましたので、こちらのissueについても検討を再開できるかなと思うのですが、いかがでしょう？
もちろん、起票からだいぶ時間が経っているので、改めて検討し直したいというのでもいいですし、ディレクトリ構成が見直されただけでも私は快適に感じているので、無理にこちらのissueの前進を勧めるものではありません。 @y-chan さんが気になった時に気になったところだけ手を入れたい、というのでも大丈夫です。とにかく無理はなきように。

[y-chan]

@kaizumaki メンションありがとうございます。
かなり時間が経っているのは事実ですし、ディレクトリ構成の見直しでかなり改善されたというのは私も思っているので、改めて検討し直す方が良さそうですね。

そもそも統廃合が必要であるか、から考え直した方が良さそうです。
個人的にはディレクトリ構成の見直しによって、逆に統廃合が難しくなっているようにも感じるので、見送りと言うのも割と視野に入れるべきかなとは思っています。
少し考えて見ますね。