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

Warning validateDOMNesting in case toast message contains <div> #5

Closed
ambroseus opened this issue Dec 21, 2020 · 8 comments
Closed
Labels
bug Something isn't working

Comments

@ambroseus
Copy link

ambroseus commented Dec 21, 2020

Got warning in Chrome: validateDOMNesting(...): <div> cannot appear as a descendant of <p>.
with material-ui's <Alert> inside toast message (or any other div)

Possible solution: replace 'p' with 'div'

const Message = styled('p')`

@timolins
Copy link
Owner

Thanks, good catch. Will fix this with the next release.

@albannurkollari
Copy link

Replacement is never a good choice! What if the developer has specific styling to just that <p> element?

I'd rather have the library throw an error (or at least log as error and not render the toaster at all) than modify the input from the dev.

Remember, if you silently fix a mistake from a developer you inadvertly are not helping them and thus they are stuck with that knowledge until someone points it out for them.

Best choice would be if you provide an option for silently logging errors instead of throwing it in such scenarios, if the user wants to pass it as an argument.

@ambroseus
Copy link
Author

ambroseus commented Dec 21, 2020

yep, also in case of default styled <p> it's not possible to easy style custom toast such as <Alert> from material-ui, for ex.
Selection_009

@ambroseus
Copy link
Author

It will be good to have the ability to override default toast container (<Message> component)

@timolins
Copy link
Owner

@albannurkollari Not sure if I got that correctly, but I think by replacing @ambroseus was referring to using div by default instead of p. This shouldn't have too many drawbacks other than being a bit less semantic. The error itself wasn't thrown by react-hot-toast, but by React itself because it renders incorrect HTML (nesting a div inside a p tag).

That said I agree that it should be possible to render a custom component, without having to go headless. Not sure yet what's the best API here since there are two components (ToastBar + Message) that need to be replaced. Maybe a disableStyles option would do the trick as well.

@ambroseus
Copy link
Author

@timolins maybe something like this (override Message component at all):

  <Toaster toastOptions={{
      MessageContainer: (props) => <div {...props} />
  }} />

@albannurkollari
Copy link

@timolins Thanks for clarification, I wasn't aware that fhe context was for the default wrapper of the Toaster! And ofc, the error/warning comes from React.

@timolins timolins added the bug Something isn't working label Dec 21, 2020
@timolins
Copy link
Owner

I'll open a new issue regarding the message customisation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants