mui-x icon indicating copy to clipboard operation
mui-x copied to clipboard

[pickers] Support slots on the `DateField` component

Open flaviendelangle opened this issue 3 years ago • 6 comments

Which slots do these components have

Slots of single input fields

Components: DateField, soon to come TimeField and DateTimeField, SingleInputDateRangeField

In theory, this component has no HTML tag of its own, it just calls the input with some custom props. This input can for instance be a TextField which already has 2 wrapping div (one in TextField and one in InputBase).

I see 3 possible approaches:

  1. Root + Input: Add a not very useful div around this input just to have a Root slot and name the slot holding the TextField Input (like described in #4484)

  2. Input: Don't add any div and name the slot holder the TextField Input (like described in #4484)

  3. Root: Don't add any div and name the slot holder the TextField Root. This is the most rigorous for the slot nomenclature of the component but it's very incoherent with the multi input fields. I think we would loose our users if we start to name slots with the same concept differently depending on the component holding them. It would be a nightmare to document.

Slots of multi input fields

Components: MultiInputDateRangeField

  • Root
  • Input (used for both the start and end input but users can use the callback for of componentsProps to customize differently the two, this will need to be documented)
  • Separator

flaviendelangle avatar Sep 06 '22 16:09 flaviendelangle

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 501.9 1,001.9 626.2 749.3 188.049
Sort 100k rows ms 545.1 1,107.1 545.1 892.46 205.456
Select 100k rows ms 169 307.7 269 255.44 46.916
Deselect 100k rows ms 113.4 290.6 201.2 210.96 59.996

Generated by :no_entry_sign: dangerJS against c4958bc998402acf4b55d42d315f479dc98e30f4

mui-bot avatar Sep 06 '22 16:09 mui-bot

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Sep 09 '22 09:09 github-actions[bot]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Sep 15 '22 09:09 github-actions[bot]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Sep 19 '22 07:09 github-actions[bot]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Sep 19 '22 16:09 github-actions[bot]

More in favor of option 2, because I'm not a big fan of adding useless div. And it makes sense that the "multi input fields" has the same slot plus a wrapper (Root) and a Separator.

alexfauquette avatar Sep 21 '22 08:09 alexfauquette

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Sep 23 '22 15:09 github-actions[bot]

We discussed it with @joserodolfofreitas, it feel like we all agree that option 2 (see PR description) is the best here

flaviendelangle avatar Sep 26 '22 06:09 flaviendelangle

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Sep 27 '22 08:09 github-actions[bot]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Sep 27 '22 10:09 github-actions[bot]