Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calloutコンポーネントに閉じるボタンを追加します #50

Merged
merged 4 commits into from
Jan 22, 2025

Conversation

changmaru
Copy link
Contributor

@changmaru changmaru commented Jan 20, 2025

snackbarコンポーネントを参考に、calloutコンポーネントにcloseボタンを追加します。

bodyが1行だった場合に上下方向のズレが発生しますが、複数行になった際を考慮し上下中央揃えは追加せず、利用する際に追加してもらう方針としました。

1行 複数行
スクリーンショット 2025-01-20 19 43 11 スクリーンショット 2025-01-20 19 44 14
スクリーンショット 2025-01-20 19 43 54 スクリーンショット 2025-01-20 19 44 35

@changmaru changmaru changed the title Callout Calloutコンポーネントに閉じるボタンを追加します Jan 20, 2025
@changmaru changmaru self-assigned this Jan 20, 2025
Copy link
Collaborator

@fumink7 fumink7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

良さそうに思いましたが1点相談的なコメントしてます!

Comment on lines 43 to 49
&:active,
&.--active {
visibility: visible;
opacity: 100%;
translate: 0 0;
transition: all 0.4s;
}
Copy link
Collaborator

@fumink7 fumink7 Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

callout、表示されているのが基本のような気がするので、むしろ.--hiddenで非表示になるとかの方がいいのかも…?と思いましたがどうなんでしょうね?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mtgでお話ししました

今回の変更ではボタンを追加するスペースを設定する、ということが目的でした。
calloutを非表示にするロジックについては利用者に委ねる方針に決定したため、関連スタイルは不要となりました。

<div className={wrapperClasses.join(' ')}>
<div
className={wrapperClasses.join(' ')}
aria-live="polite"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この属性が必要かどうかは、trailingに入るボタンによる効果に依るように見受けられるので、もしそうなら不要かなと思いました

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

75e90b0 の削除忘れでした
ありがとうございます!

Copy link
Contributor

@piyoppi piyoppi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙆‍♂️

Copy link
Collaborator

@fumink7 fumink7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます!🙌

@changmaru changmaru merged commit 76ac99c into main Jan 22, 2025
2 checks passed
@piyoppi piyoppi deleted the callout-closebutton branch January 23, 2025 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants