[refactor] アップデート通知ダイアログ周りをEditorHome.vueから分離 #1717
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hiroshiba
commented
Jan 17, 2024
Comment on lines
+58
to
+63
| "vue/multi-word-component-names": [ | ||
| "error", | ||
| { | ||
| ignores: ["Container", "Presentation"], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
コンポーネント名は２単語以上、という制約がディレクトリ区切りのコンポーネントにも採用されてしまうので、この２つだけは許容するようにしてみました。
Comment on lines
+20
to
+32
| const wrapper = mount(UpdateNotificationDialogPresentation, { | ||
| props: { | ||
| modelValue: true, | ||
| latestVersion, | ||
| newUpdateInfos: [], | ||
| onSkipThisVersionClick, | ||
| }, | ||
| global: { | ||
| plugins: [Quasar], | ||
| }, | ||
| }); | ||
| await flushPromises(); | ||
| const domWrapper = new DOMWrapper(document.body); // QDialogを取得するワークアラウンド |
There was a problem hiding this comment.
普通にmountしてもQDialogがなぜかうまいこと動かないのでワークアラウンド。
Comment on lines
+45
to
+50
| describe("Presentation", () => { | ||
| enableAutoUnmount(afterEach); | ||
|
|
||
| it("マウントできる", async () => { | ||
| mountUpdateNotificationDialogPresentation(); | ||
| }); |
There was a problem hiding this comment.
表示だけなのでコンポーネントを単体テストできる！
たぶんStoryBookとかも導入できる。
|
多分問題ないと思うのでマージします！ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
内容
アップデート通知ダイアログ周りの処理を、EditorHomeから移動させてみました。
ついでに見た目側の単体テストも書いています。
設計はContainer/Presentationalパターンです。その中でも（↓で書いてある）分割パターンです。
https://zenn.dev/buyselltech/articles/9460c75b7cd8d1
ロジックを分離するだけならコンポーザブルで良く、実際書いてみたのですが、UIの方にpropsを渡すために加工する部分がごちゃついてしまいました。
なのでAPIやVuexを叩いてpropsを加工するだけのContainerを導入してみたら、まあまあ良い感じになりました。
関連 Issue
その他
Containerを含めた複合テストも書いてみようとしたのですが、思った以上に難しかったです。
Vuexを導入する必要がある（これはそんなに難しくない）のと、あとアプリバージョンを取得する周りのmockが綺麗に作れないなと。
Container含めた制御ロジックはテストしづらいので、なるべくロジックを書かないのが良さそうに感じました。
今回のコードは、２つのバージョンを比較してより新しい方を選ぶ部分だけロジックが入ってます。