alexfauquette commented 1 year ago

Problem to solve

The pickers components share a lot of properties/behavior. A good example is the validation feature. All components share it, but not exactly the same if its a Date, Time or DateTime component.

To explain the feature once and allows to experiment with different components, we build demonstrations that show multiple components at once.

The problem is that displaying those components in a pleasing way that works for both mobile and desktop takes time. In order to help in this effort, it has been introduced two componentsDemoContainer and DemoItem responsible to adapt the layout depending on their children.

The solution might be a tradeoff between:

simplifying demos creation
demos should work in standalone (codesandbox)
demos and especially previews should be easy to read (not too verbose)

Already tried

We already tried multiple solutions and faced some issues

Broken sandbox

The first attempt was to put DemoContainer and DemoItem in the docs package. This breaks codesandbox, because docs is not exported, so condsandbox can not import them.

The solution has been to move those components to the internals.

Broken build

A second point was the use of displayName property of React object. But they are not available in the build. Lukas tried to add them, it move package weight from 404kB to 457kB (more than 10% increase for something developers will not use)

The current approach is to provide a prop components to DemoContainer with the list of all the pickers elements it includes, such that it can adapt its styling to them. And a prop component (without "s") which receives a string with the component it wrapps. This props can be undefined if the default behavior of DemoItem is ok. For now, we only need to specify component when it contains a range picker with multi inputs. To help populate those props, we have a codemod that can be run with

yarn docs:populate:demos

To discuss

Pedagogical aspect of the demonstration

The problem with those solutions is the noise they bring around examples. Here is an example:

// The demonstration
<LocalizationProvider dateAdapter={AdapterDayjs}>
  <DemoContainer components={['TimePicker', 'DateTimePicker']}>
    <DemoItem label="TimePicker">
      <TimePicker defaultValue={nineAM} maxTime={fiveAM} />
    </DemoItem>
    <DemoItem label="DateTimePicker">
      <DateTimePicker defaultValue={nineAM} maxTime={fiveAM} />
    </DemoItem>
  </DemoContainer>
</LocalizationProvider>

// The interesting part
<LocalizationProvider dateAdapter={AdapterDayjs}>
      <TimePicker defaultValue={nineAM} maxTime={fiveAM} />
      <DateTimePicker defaultValue={nineAM} maxTime={fiveAM} />
</LocalizationProvider>

Having to find the information between all those <Demo...> can be discouraging and they do not bring value to users since they are internal stuff.

Another option would be to directly use Stack which at least is useful for users since they can reuse them.

Or we decide that some demonstrations are more for interaction and others to do some copy past

User experience improvement

Idea from José:

On follow-ups, we can consider improving the DX to copy+paste the code. With multiple components+/internals/demo, the code showcase feels a bit cluttered. Maybe we can explore mitigating the trade-offs of displaying multiple components, by making it easier to copy the important parts of the demo.

Perhaps something like (this quick idea)

flaviendelangle commented 1 year ago

My main problem with a Stack implementation is that we would end up with the responsive rules redefined on each demo, and they would have to differ depending on the content (e.g: you can put 2 DateField on the same line on xl, not on l but for SingleInputDateRangeField you can't ever put 2 on the same line).

These rules could evolve if we change the width of the demo container in the future or if we redesign some of our components. So for me this would be a nightmare to correctly maintain and we would end up with the same situation we had before DemoContainer: the demo works well on resolutions that we are using on a day to day basis, but with tons of overflows when you start using it on smaller screens.

If we want to avoid the DemoContainer complexity altogether, I would be in favor of always using direction="column" except on very simple examples.