feat(compass-components): add useToast COMPASS-5473 #2765
Conversation
mcasimir
changed the title
| <Toast | ||
| className={toastStyles} | ||
| key={id} | ||
| title={title} | ||
| body={body} | ||
| variant={variant} | ||
| progress={progress} | ||
| open={true} | ||
| close={() => closeToast(id)} | ||
| /> |
There was a problem hiding this comment.
Seems like by default toast component renders itself with position fixed which means that multiple toasts are just stacked on top of each other and you can't see them:
This is how three toasts look in UI:
And this is the DOM (just to prove that there are multiple of them):
I think we either need to ask leafygreen team to implement toast stack or work around that by having our own fixed container here + adding some styles to the toast to override its position (we probably would need to do both because I don't see leafygreen implementing this change any time soon)
There was a problem hiding this comment.
fixed container here + adding some styles
That's probably easy enough and probably we could rely on the fact that position: fixed should always be on the Toast element itself.
We decided not to make this change here, but worth a try
There was a problem hiding this comment.
ok tried this out and is actually not possible to get a handle of the container for the toasts. All gets rendered by LG in a child of body, so not much we can do.
There was a problem hiding this comment.
Ah yeah, I see now that they render them in a portal (on top of having fixed position, why?) and in this case don't allow to override the portal node (or disable the portal similar to other components). I will ask them how are we even supposed to handle this. If you want to merge this as-is that's fine, but basically with how leafygreen works right now we can't make multiple toasts work on the same screen and so maybe we shouldn't even "allow" this on the hook level, because we are basically allowing broken behavior that way
There was a problem hiding this comment.
At hook level is partially handled, I think is best if LG fixes it, but since you have to pass an id, creating multiple toasts has to be intentional. Ie. the copy-connection-string will not create multiple toasts, but the next one will replace the previous one if is there.
There was a problem hiding this comment.
Yeah, we definitely handle it on our side! What I mean is what happens when I need to add a toast with another id somewhere else in the app? It will result in a broken UI with overlapping toasts, do we just agree that this is okay for now and wait for leafygreen team to provide the solution in the future?
Co-authored-by: Sergey Petushkov <petushkov.sergey@gmail.com>
mcasimir
force-pushed
the
COMPASS-5473
branch
from
b2c15ee to
87ae53d
Compare
There was a problem hiding this comment.
Looks good! I'm concerned about this #2765 (comment) and would prefer to decide how we should handle this before this PR lands, but also don't want to block it on this
2 participants
Replace state-based toasts with a global provider + hooks
Feb-07-2022.21-19-28.mp4