-
Notifications
You must be signed in to change notification settings - Fork 139
fix(dialog): moved close button to the top of dialog in dom, marked caption in dialog header as h2 #2561
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
base: main
Are you sure you want to change the base?
Conversation
|
@PosikBoy Could u provide more detailed information in PR title? |
|
Preview is ready. |
|
🎭 Component Tests Report is ready. |
It was like draft) Added detailed information in title |
| <div className={b(null, className)}> | ||
| {insertBefore} | ||
| <div className={b('caption')} id={id}> | ||
| <h2 className={b('caption')} id={id}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use Text component and add as field to DialogHeaderProps? And return div as default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Please update the PR title to reflect the last changes
| <div className={b(null, className)}> | ||
| {insertBefore} | ||
| <div className={b('caption')} id={id}> | ||
| <Text as={as} className={b('caption')} id={id}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use variant prop subheader-3 instead of setting font styles in CSS
|
@PosikBoy Is it best practice to put close button first in tab order? |
Closes #2560