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

The demos should work in isolation #1650

Closed
oliviertassinari opened this issue Apr 12, 2020 · 8 comments
Closed

The demos should work in isolation #1650

oliviertassinari opened this issue Apr 12, 2020 · 8 comments
Labels
Milestone

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 12, 2020

Environment

Tech Version
@material-ui/pickers v4.0.0-alpha.5

Steps to reproduce

  1. Go to https://next.material-ui-pickers.dev/demo/datepicker
  2. Copy and paste the source in codesandbox
  3. It crashes

Can not find utils in context. It looks like you forgot to wrap your component in LocalizationProvider, or pass dateAdapter prop directly.

Expected behavior

The demos should all be working in isolation, no matter the cost. I think that it's a critical point required to minimize churn during the first few minutes a developer try this option (competing libraries) out.

@dmtrKovalenko
Copy link
Member

What do you prefer? Wrap each demo in LocalizationProvider or pass dateAdapter prop?

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Apr 13, 2020

I think that we should have both:

  • Have LocalizationProvider
  • Create the utils from the most popular lib, likely date-fns.

Regarding testing with different date engines, I imagine it's much more a library maintainer concern than a developer concern. However, I think that the demos could accept an optional util prop coming from the outside.

Would it work? My only concern is with the overhead of these the demos. But I think that it's healthy, it incentive us to make configuration as easy as possible for developers.

@oliviertassinari
Copy link
Member Author

We will also need to remove all imports from docs folders, for instance

import { makeJSDateObject } from "../../../utils/helpers";

@dmtrKovalenko
Copy link
Member

I mean using context provider in each example or pass dateAdapter right to the pickers? https://next.material-ui-pickers.dev/guides/date-adapter-passing

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Apr 13, 2020

@dmtrKovalenko What about this structure?

import React from "react";
import LocalizationProvider from "@material-ui/pickers/LocalizationProvider";
import DesktopDatePicker from "@material-ui/pickers/DesktopDatePicker";
import dateAdapter from "@material-ui/pickers/adapters/date-fns";

function withContext(Component) {
  return (props) => (
    <LocalizationProvider dateAdapter={props.dateAdapter || dateAdapter}>
      <Component />
    </LocalizationProvider>
  );
}

function AdvancedKeyboardExample(props) {
  const [selectedDate, handleDateChange] = React.useState(new Date());

  return (
      <DesktopDatePicker
        autoOk
        variant="outlined"
        label="Advanced keyboard"
        placeholder="2018/01/01"
        inputFormat="yyyy/MM/dd"
        mask="____/__/__"
        value={selectedDate}
        onError={a => {
          console.log("error", a);
        }}
        onChange={date => handleDateChange(date)}
      />
  );
}

export default withContext(AdvancedKeyboardExample);

@oliviertassinari oliviertassinari added this to the v5 milestone May 2, 2020
@oliviertassinari
Copy link
Member Author

oliviertassinari commented May 2, 2020

mui/material-ui#20878 made me think of this issue, adding the v5 label per the internal discussion we had around:

  1. v4 => solid foundations for desktop date pickers (a11y, material design, UI/UX, bugs)
  2. v5 => focus on the documentation + integration with the main mono-repository (docs++, SEO, l10n, etc)

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jul 16, 2020

A counter-proposal plan to #1650 (comment) for this:

  1. In the adapter page of the documentation, we add a full demo with each date adapter.
  2. We are pragmatic, we will aim for the 80/20 solution. date-fns is, by a large margin, the most used date engine in the community: 54% and growing. We use it for all the demos in the documentation. People would have to do a bit more legwork for using a different date adapter.
  3. We give up on the adapter switcher in the documentation. We don't really need this feature. Who uses it?

@oliviertassinari
Copy link
Member Author

It was fixed in https://next.material-ui.com/components/pickers/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants