QuestionDialogの文字色を修正、StorybookのVRTを改善 #2314
Merged
+109
−16
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
[update snapshots]
sevenc-nanashi
requested review from
Hiroshiba
and removed request for
a team
[update snapshots]
[update snapshots]
[update snapshots]
[update snapshots]
[update snapshots]
sevenc-nanashi
changed the title
[update snapshots]
[update snapshots]
[update snapshots]
[update snapshots]
Hiroshiba
reviewed
Oct 23, 2024
Hiroshiba
reviewed
Oct 24, 2024
Comment on lines
75
to
90
| const root = page.locator("#storybook-root"); | ||
| const dialogRoot = page.locator("div[id^=q-portal--dialog--]"); | ||
| const firstVisible = await Promise.race([ | ||
| root.waitFor({ state: "visible" }).then(() => "root"), | ||
| dialogRoot.waitFor({ state: "attached" }).then(() => "dialog"), | ||
| ]); | ||
|
|
||
| // ダイアログの場合はダイアログのスクリーンショットを、そうでない場合は#storybook-rootのスクリーンショットを撮る。 | ||
| // q-portal-dialogはそのまま撮るとvisible扱いにならずtoHaveScreenshotが失敗するので、 | ||
| // 子要素にある実際のダイアログ（.q-dialog）を撮る。 | ||
| if (firstVisible === "dialog") { | ||
| const dialog = dialogRoot.locator(".q-dialog"); | ||
| await expect(dialog).toHaveScreenshot(`${story.id}-${theme}.png`); | ||
| } else { | ||
| await expect(root).toHaveScreenshot(`${story.id}-${theme}.png`); | ||
| } |
There was a problem hiding this comment.
かなりライブラリの実装依存なので、後から修正できるように丁寧に作っておきたいですねぇ。
- Quasarは後々依存をやめると踏んで、ダイアログ→Quasarダイアログに呼び方を一部変える
- 最初にダイアログ用かどうかで全コードを分岐するようにする
- たぶん
firstVisible変数は無くせる（dialogRootの有無で分岐できる）
- たぶん
感じにしてみるのはどうでしょう。
There was a problem hiding this comment.
雰囲気こんな感じのコード？
// Quasarダイアログの場合はダイアログのスクリーンショットを、そうでない場合は#storybook-rootのスクリーンショットを撮る。
const root: Locater;
const dialogRoot = page.locator("div[id^=q-portal--dialog--]");
if (dialogRootがある) {
// q-portal-dialogはそのまま撮るとvisible扱いにならずtoHaveScreenshotが失敗するので、
// 子要素にある実際のダイアログ（.q-dialog）を撮る。
await dialogRoot.waitFor({ state: "attached" })
root = dialogRoot.locator(".q-dialog")
}
else {
await root.waitFor({ state: "visible" })
root = page.locator("#storybook-root");
}
await expect(root).toHaveScreenshot(`${story.id}-${theme}.png`);
Co-authored-by: Hiroshiba <[email protected]>
Hiroshiba
reviewed
Oct 24, 2024
[update snapshots]
[update snapshots]
[update snapshots]
スマホ編集なんてやるもんじゃないな… [update snapshots]
Hiroshiba
approved these changes
Oct 24, 2024
Hiroshiba
reviewed
Oct 24, 2024
There was a problem hiding this comment.
あれ、そういえばダークモードだとボタンがprimary colorなんですね
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.
内容
タイトル通りです。
VRTは以下のような変更を行っています：
関連 Issue
スクリーンショット・動画など
（なし）
その他
（なし）