-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[WIP] Add support for Bootstrap 4.2's Toasts feature (unfinished PR. feedback welcome) #1384
Conversation
}, | ||
}; | ||
|
||
function Alert(props) { |
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.
Change Alert
to Toast
(here and below)
const classes = mapToCssModules(classNames( | ||
className, | ||
'toast', | ||
body ? 'toast-body' : false, |
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.
Is this a thing: .toast.toast-body
? I didn't notice any examples where those classes were on the same element.
return ( | ||
<WrapTag {...attributes} className={classes}> | ||
{icon && ( | ||
<svg |
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.
I think this svg was just for the examples in the bootstrap docs. It would be good to allow the user to specific something custom.
Though, I do like the idea of specifying a color to have a square of that color. Maybe allow a component or a string. In the case of a component, the component is used as is. When it is a string, this svg can be used.
@@ -1,3 +1,5 @@ | |||
import ToastHeader from "./ToastHeader"; |
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.
You're importing the ToastHeader
? Why?
|
||
toggle() { | ||
this.setState({ | ||
show: !this.state.show |
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.
better to use prevState here.
I was just looking to add this feature in and found this open PR. I was wondering if we could add in an additional prop such as Either this can be added or we can do something like the below: Create a <Toaster position="top-right">
..Normal Application
</Toaster> Then export a const toast = useToaster(); Then in any underlying component you can call the toaster to create by just using the Wanted to get everyone's thoughts on this and possibly add this in as a feature after this has been merged in. |
Any progress on this? Would be wonderful to get this deployed. |
@samuelcolvin I haven't had any time to work on this, but anyone is welcome to pick up where I left off. I think I did much of the gruntwork of putting together the right files, but there needs to be much more finessing. |
I just opened #1447 to address the feedback on this PR and hopefully get it over the line. |
WIP for issue #1346
Screenshot of progress: https://user-images.githubusercontent.com/1542740/51679897-2eb69900-1fae-11e9-85eb-01a7ba9242de.png
--