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

[docs] Add material-ui-snackbar-provider integration demo #17762

Conversation

leMaik
Copy link
Member

@leMaik leMaik commented Oct 6, 2019

@oliviertassinari asked What do you guys think of an imperative API to display a snackbar?

Turns out that we (at Wertarbyte) have been maintaining just that for years. 😅 This PR adds an integration example (JS and TS).

The library doesn't support queueing snackbars yet, though. That would be pretty neat, but the imperative API alone is nice. In fact, I never used the snackbar without it since 2017.

@leMaik leMaik added docs Improvements or additions to the documentation component: snackbar This is the name of the generic UI component, not the React module! labels Oct 6, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Oct 6, 2019

No bundle size changes comparing fb0b3d0...385618b

Generated by 🚫 dangerJS against 385618b

@leMaik
Copy link
Member Author

leMaik commented Oct 6, 2019

BTW: I had to add trim-right to the docs package in order to make yarn docs:dev work. It would complain about not finding that package otherwise. 🤔

@joshwooding
Copy link
Member

BTW: I had to add trim-right to the docs package in order to make yarn docs:dev work. It would complain about not finding that package otherwise. 🤔

I had that issue but after clearing everything down and reinstalling it worked fine.

@oliviertassinari
Copy link
Member

@leMaik What do you think of exploring APIs we can expose in the core rather than relying on a third package?

@leMaik
Copy link
Member Author

leMaik commented Oct 6, 2019

@oliviertassinari I'd love to see a similar API in core, but that's orthogonal to this PR for now. Using a third-party package is a nice way to see how users want to use it and have something until it lands in core (similar to the way we provided a rating component way before it went into the lab).

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 7, 2019

I'd love to see a similar API in core, but that's orthogonal to this PR for now. Using a third-party package is a nice way to see how users want to use it and have something until it lands in core (similar to the way we provided a rating component way before it went into the lab).

@leMaik Interesting, from my perspective, notistack has served this purpose so far. I think that we can jump the need validation step and move to the next step: the lab. I think that we have strong enough signals: react-toastify, antd notifications, notistack, Chakra useToast, 2 or 3 feedback I have received chating with users, my personal frustration when facing similar challenges, and your material-ui-snackbar-provider project.

Regarding the implementation, from what I understand, the use case for the theme provider is about providing a global configuration. I hope we can use the theme.props.MuiX feature instead.

@leMaik
Copy link
Member Author

leMaik commented Oct 7, 2019

Regarding the implementation, from what I understand, the use case for the theme provider is about providing a global configuration. I hope we can use the theme.props.MuiX feature instead.

It's for providing props to the Snackbar, to configure it. It feels weird to do that via the theme.

As I already said in gitter: What's good about the container is that you can nest them and configure them individually. E.g. I have apps with a BottomNavigation bar on some views and a different nested SnackbarProvider that changes the snackbar to have a bottom margin so that it displays on top of the bottom bar.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 7, 2019

We have a similar requirement for 1. the date picker, people would need to configure the locale, and potentially the date engine backend cc @dmtrKovalenko 2. the useMediaQuery engine on the server. 3. the removal of the ripple globally. So far, the theme has been the encouraged path. An alternative would be to consider a ConfigProvider component that can be shared between components. Would it it be significantly better? Is the distinction strong enough?

@leMaik
Copy link
Member Author

leMaik commented Oct 7, 2019

@oliviertassinari That would require the theme to inject the component. Somehow the snackbar needs to be actually displayed. It shouldn't do that and neither should a shared configuration provider. What if the user doesn't even use the snackbar? It would be bundled anyway because the theme/shared config component uses it.

Also, that approach wouldn't solve the use case I mentioned in my comment above.

@oliviertassinari
Copy link
Member

It would be bundled anyway because the theme/shared config component uses it.

Take the useMediaQuery, the hook pulls the options from theme.props.MuiUseMediaQuery. The bundle size implication is very limited (only the cost of useTheme and getThemeProps).

@oliviertassinari oliviertassinari added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI new feature New feature or request and removed docs Improvements or additions to the documentation labels Oct 9, 2019
@leMaik
Copy link
Member Author

leMaik commented Oct 9, 2019

@oliviertassinari Should I update this PR to start adding an imperative API in the lab (regarding the label changes)?

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 9, 2019

@leMaik I have changed the labels to signal that I think that adding a new alternative project in the documentation wouldn't be the best path forward.

Looking at https://www.npmjs.com/search?q=keywords%3Amaterial-ui&ranking=popularity I think that notistack (from @iamhosseindhv) is a successful effort.

I really like the imperative API for this component. We could consider add a mention about it in the documentation: https://material-ui.com/components/snackbars/#notistack.

I see two possible paths going forward. We either 1. double down on notistack (like propose to move it to the mui-org) or 2. we solve some of the problems directly in the core (it doesn't have to be at 100% but we can move the good parts), for instance, rollout the danger, info, success states to all the MUI components, add an imperative API (as we have dicussed), etc.

My personnal belief is that, this is not a high priority. Notistack has 8% of the MUI downloads, which sounds high for the scope of the problem solved. In https://www.npmjs.com/search?q=keywords%3Amaterial-ui&ranking=popularity I see problems MUI core don't solve well that third party libraries struggle to fill. I think focusing on the problems we (the MUI ecosystem) don't solve really really well would be better. For instance with the components, I can think of: date picker, datatable, upload, carousel, dropdown, pagination, alert.

What do you think?

cc @mui-org/core-contributors

@leMaik
Copy link
Member Author

leMaik commented Oct 10, 2019

I think focusing on the problems we (the MUI ecosystem) don't solve really really well would be better. For instance with the components, I can think of: date picker, datatable, upload, carousel, dropdown, pagination, alert.

I agree, notistack really solves this problem. Having the imperative api in core would be nice to have, though.

  • Date picker: done (?) in @mui-org/material-ui-pickers
  • Datatable: I think a generic data-table for every use case is very hard to get right; I, for one, like https://www.material-ui-datatables.com/ but it misses some ways to filter (and column reordering, and other things) we needed for a project, so we ended up implementing a custom data-table (not published as of now, though). Neither of the two would solve all use cases.
  • Upload: We needed this and experimented a bit at https://github.com/TeamWertarbyte/material-ui-file-dropzone, works fine but didn't really gain much traction, though. I recently implemented a <input type="file" /> replacement on top of this, might be a thing to add a demo for, but I don't know if this should be in core.
  • Carousel: Same as datatable; there are too many use cases and I'm not sure if there is any common divisor that Material-UI could implement. It's pretty much react-swipeable-views+images most of the time

@oliviertassinari
Copy link
Member

notistack really solves this problem. Having the imperative api in core would be nice to have, though.

@leMaik Yeah, during the first years of the project, I hoped that we could build a slim core and get many third party projects fills the holes (in terms of what developers need). But the more we move forward, the less practical this option seems. I have observed a push of the community toward a concentrated set of official approaches (from @material-ui/core. I think that part of the reason why is that our audience wants a single product they can install, and they are done. I can understand that! For instance, I wish that React was providing an official styling solution. It would make our mission so much simpler :).

The incentive for developers toward concentration seems to be:

  1. Solve the paradox of choice, it's simpler to pick from one option than many.
  2. Solve the community critical mass problem, a single solution with more users can solve more problems, reach a better level of quality.
  3. Solve the entropy issue with many dependencies, you have to upgrade them all, with no guarantee of interoperability.
  4. Solve the documentation fragmentation issue, it's simpler to browse one documentation that uses the same set of conventions for all its modules.
  5. Solve the bundle size issue, because you enforce a limited set of dependencies, you can make sure there is no transitive dependencies overlap

Now, all these fundamental issues could be solved without actually having mui-org/material-ui "own" the source. I think that for each point, we can imagine solutions. But for some reason, people push for it. I suspect a psychological reason, like, it reduces risks. The last example of it is the survey for the data picker, early results show: "moving the project to the core would greatly help". Well, if it harms the incentive for the main maintainer to work on the project, probably not 🙃.

So regarding notistack itself, the longer it can stay a third-party project and thrive, I think the better. Maybe we could look at the topic again in 1 year from now, with a fresh eye?

Date picker: done

I wish it was the case, not yet :). I would imagine that it needs between 100-500 hours to set it on a new path: mui/material-ui-pickers#1293. And then of course, continuous maintenance.

Datatable

Thanks for this feedback. Yeah, there is a significant need for this, I agree. It's also an opportunity for Material-UI to really (more efficiently than we do now) monetize (with an advanced paid version of it), and hopefully, help grows the project faster. One thing to consider: https://datatables.net is very popular with jQuery (traffic from Wordpress users).

Upload

This one is on my shortlist :). I think that it's something we should look at in the near future. Regarding the link you sent, I can imagine that it needs significantly more features. Also, I think that the idea would also be to provide our own version of react-dropzone. If we want to implement Bootstrap, or Antd, or different themes in the future, we need to go down this path of almost zero dependencies.

Carousel

Agree, this one seems less a priority: https://twitter.com/MaterialUI/status/1148901411180163073. I would be in favor of ignoring it for the next few months.

(These components are listed in the Roadmap)

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think of the change to the markdown I'm proposing?

@oliviertassinari
Copy link
Member

diff --git a/docs/src/pages/components/snackbars/IntegrationNotistack.js b/docs/src/pages/components/snackbars/IntegrationNotistack.js
index 10092d468..8723385bd 100644
--- a/docs/src/pages/components/snackbars/IntegrationNotistack.js
+++ b/docs/src/pages/components/snackbars/IntegrationNotistack.js
@@ -11,13 +11,13 @@ function MyApp() {

   const handleClickVariant = variant => () => {
     // variant could be success, error, warning, info, or default
-    enqueueSnackbar('This is a warning message!', { variant });
+    enqueueSnackbar('This is a success message!', { variant });
   };

   return (
     <React.Fragment>
       <Button onClick={handleClick}>Show snackbar</Button>
-      <Button onClick={handleClickVariant('warning')}>Show warning snackbar</Button>
+      <Button onClick={handleClickVariant('success')}>Show success snackbar</Button>
     </React.Fragment>
   );
 }
diff --git a/docs/src/pages/components/snackbars/IntegrationNotistack.tsx b/docs/src/pages/components/snackbars/IntegrationNotistack.tsx
index ef2144a81..e6f3b61bd 100644
--- a/docs/src/pages/components/snackbars/IntegrationNotistack.tsx
+++ b/docs/src/pages/components/snackbars/IntegrationNotistack.tsx
@@ -11,13 +11,13 @@ function MyApp() {

   const handleClickVariant = (variant: VariantType) => () => {
     // variant could be success, error, warning, info, or default
-    enqueueSnackbar('This is a warning message!', { variant });
+    enqueueSnackbar('This is a success message!', { variant });
   };

   return (
     <React.Fragment>
       <Button onClick={handleClick}>Show snackbar</Button>
-      <Button onClick={handleClickVariant('warning')}>Show warning snackbar</Button>
+      <Button onClick={handleClickVariant('success')}>Show success snackbar</Button>
     </React.Fragment>
   );
 }
diff --git a/docs/src/pages/components/snackbars/snackbars.md b/docs/src/pages/components/snackbars/snackbars.md
index 8e466a215..9c79382da 100644
--- a/docs/src/pages/components/snackbars/snackbars.md
+++ b/docs/src/pages/components/snackbars/snackbars.md
@@ -76,8 +76,8 @@ For more advanced use cases you might be able to take advantage of:
 ![stars](https://img.shields.io/github/stars/iamhosseindhv/notistack.svg?style=social&label=Stars)
 ![npm downloads](https://img.shields.io/npm/dm/notistack.svg)

-In the following example, we demonstrate how to use [notistack](https://github.com/iamhosseindhv/notistack).
-notistack makes it easy to display snackbars (so you don't have to deal with open/close state of them).
-It also enables you to stack them on top of one another (although this is discouraged by the specification).
+We demonstrate how to use [notistack](https://github.com/iamhosseindhv/notistack).
+notistack has an **imperative API** that makes it easy to display toasts (so you don't have to deal with open/close state of them).
+It also enables you to **stack** them on top of one another (although this is discouraged by the specification).

 {{"demo": "pages/components/snackbars/IntegrationNotistack.js"}}

@leMaik
Copy link
Member Author

leMaik commented Oct 15, 2019

@oliviertassinari I don't see how the markdown changes change much. The existing demo was just fine. 👍

Regarding the file upload component: I'd really like to work on that one, but we'll need to think about the features it should include.

Also, I think that the idea would also be to provide our own version of react-dropzone. If we want to implement Bootstrap, or Antd, or different themes in the future, we need to go down this path of almost zero dependencies.

The link I sent to you actually has zero dependencies and doesn't use react-dropzone.

@oliviertassinari
Copy link
Member

Regarding the file upload component: I'd really like to work on that one, but we'll need to think about the features it should include.

Great! I think that we should start with a benchmark of all the most used and popular implementations of the upload use case, including proprietary software solutions in the list. I didn't know it has no dependencies. I haven't looked close enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: snackbar This is the name of the generic UI component, not the React module! new feature New feature or request PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants