-
Notifications
You must be signed in to change notification settings - Fork 645
Extract Dialog header into separate component #736
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
Conversation
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/primer/primer-components/664ijgvj2 |
colebemis
left a comment
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.
Nice, @BinaryMuse! I like this API a lot better ❤️
emplums
left a comment
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.
Just left a couple comments!
|
|
||
| const Dialog = ({title, children, ...props}) => { | ||
| function DialogHeader({theme, children, ...rest}) { | ||
| if (React.Children.toArray(children).every(ch => typeof ch === 'string')) { |
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.
🙌
…der-extraction" Extract Dialog header into separate component - (Edit) add commit be6ed3
This PR extracts the
Dialogheader into a separate component, thus making it optional and more customizable. It also shifts default props around so that default styling is not affected, but users can still customize the background and flex properties of the header.Since the original incarnation had specific styling for string titles, I've extracted the header into two components:JK this is fixed.Dialog.Headerfor fully custom headers, andDialog.HeaderTextfor headers with only text that should match the original styles. I'm not a huge fan of this solution and am open to other ideas.Finally, the close button has been placed as the first child inside
StyledDialogso that it retains initial focus when the dialog is opened.Closes #730
Merge checklist
index.d.ts) if necessaryBreaking changes
This PR introduces a breaking change.
Migration strategy
Change any
Dialogcomponent with atitleprop set as follows:to