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

Add support for Toast messages (Bootstrap 4.2.1) #1346

Closed
cvalmonte opened this issue Jan 2, 2019 · 11 comments · Fixed by #1447
Closed

Add support for Toast messages (Bootstrap 4.2.1) #1346

cvalmonte opened this issue Jan 2, 2019 · 11 comments · Fixed by #1447

Comments

@cvalmonte
Copy link

cvalmonte commented Jan 2, 2019

Please add support for toast messages which were recently added in version 4.2.1:

https://getbootstrap.com/docs/4.2/components/toasts/

@cvalmonte cvalmonte changed the title Add support for Toast messages (Bootstrap 4.2) Add support for Toast messages (Bootstrap 4.2.1) Jan 2, 2019
@bpas247
Copy link
Member

bpas247 commented Jan 8, 2019

I might open a PR if I have enough time to get around to doing this. I'm guessing we would handle showing and hiding toast messages via state?

@TheSharpieOne
Copy link
Member

I imagine they would work much like Alerts do. They are very similar in may way, just that these toasts will have more content.
I would let the use define all of the content (such as the "11 mins ago" seen in the examples), the user can pull in a library like react-timeago (or similar) his/herself if needed rather than supporting that feature and bloating reactstrap.

@bpas247
Copy link
Member

bpas247 commented Jan 8, 2019

Would it be useful to use a library like react-timeago in the Toast example? Or just reference how you would implement it via a library?

@TheSharpieOne
Copy link
Member

An example in the docs could be good, make sure when the package is installed, it's installed as a devDev.

@chez14
Copy link

chez14 commented Jan 23, 2019

Any news with this new feature?

Or just quick question, when will the docs for this feature (along with other new BS release like Switch and etc) that have been implemented be released?

@TheSharpieOne
Copy link
Member

switch and spinner have already been released (a week ago) and the docs were updated when the PRs for those features were merged (switch is on the forms page with the custom inputs and spinner has its own page).

@bpas247
Copy link
Member

bpas247 commented Jan 23, 2019

Any news with this new feature?

I haven't had the chance to implement this yet, I've been quite busy with school.

I'll open a PR once I get the first revision done.

@chez14
Copy link

chez14 commented Jan 24, 2019

Oh ok, got it. Thanks everyone xD

@hcharley
Copy link
Contributor

hcharley commented Jan 24, 2019

I started work on this in case @bpas247 hasn't yet started. Didn't want to step on toes, but I would love to see this out in the world:

https://github.com/charlex/reactstrap/tree/toasts

It's still a work in progress, but I could maybe use some suggestions on how to handle the little icon bug in the top left (and in general). Should it be a prop like icon="warning"? Or should we just not bother to support it and leave it up to lib users to implement it how they see fit?

<Toast>
 <ToastHeader icon="warning">Reactstrap</ToastHeader>
 <ToastBody>Lorem ipsum</ToastBody>
</Toast>

Here is a screenshot of the in progress docs:

https://user-images.githubusercontent.com/1542740/51679897-2eb69900-1fae-11e9-85eb-01a7ba9242de.png

@bpas247
Copy link
Member

bpas247 commented Jan 25, 2019

It's still a work in progress, but I could maybe use some suggestions on how to handle the little icon bug in the top left (and in general). Should it be a prop like icon="warning"? Or should we just not bother to support it and leave it up to lib users to implement it how they see fit?

The way you're currently implementing the icon is what I had in mind originally, so definitely keep up the good work 👍

I'd suggest opening up a PR and labeling it as [WIP] so that we can see your progress and start reviewing the diffs

@hcharley
Copy link
Contributor

Good thinking @bpas247 . I'll open a PR now.

TheSharpieOne pushed a commit that referenced this issue Apr 3, 2019
Supersedes and closes #1384. Fixes #1346.

Note that I didn't rewrite @Charlex's commit to adhere to the commit guidelines cause I didn't want to change history and make it look like I wrote that code. Hope that's alright.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants