mui / base-ui

Base UI is an open-source library of accessible, unstyled UI components for React.
https://mui.com/base-ui/
MIT License
230 stars 42 forks source link

[NumberField] onChange API / API convention? #600

Open oliviertassinari opened 6 days ago

oliviertassinari commented 6 days ago

Steps to reproduce

Link to live example: (required)

Steps:

  1. https://master--base-ui.netlify.app/components/react-number-field/

Current behavior

New API structure for the onValueChange prop.

Expected behavior

The onValueChange API signature looks different from what we used to have. I'm missing the rationale/the value for it to be different.

  1. The prop name:

https://github.com/mui/base-ui/blob/78d4abd6a72b0e3d783aa7a09bfda8c7a7518b88/packages/mui-base/src/NumberField/Root/useNumberFieldRoot.ts#L67

Shouldn't this be called onChange?

  1. The first argument, is not event. In Material UI v0, we used to do a mix of a bunch of different signatures, we always needed to expose the event, so it seemed simpler to default to expose the event first, and the value second, and optional 3rd argument, and a catch all in a 4th argument.

https://mui.com/base-ui/react-number-input/#events matches my expectations.

Overall, I think that this raises the question of API consistency. I see no reason for it to differ between MUI X, Material UI, and Base UI (the opposite, it's part of the value of doing all under a single roof, consistency in the DX). So I think it should be our aspiring goal. https://mui.com/material-ui/guides/api/#controlled-components we meant to be our source of truth up until now.

I see a few action points:

  1. Base UI needs an API page like the one of Material UI. Material UI's content should ideally not exist, refer everything back to Base UI. This should help align and scale.
  2. I think we should default to start from the one of Material UI as has the most adoption, so would minimize the breaking changes. And from there move toward Radix APIs with changes to it ⬇️:
  3. We should see how we can evolve this convention for the best. In that discussion, I would expect that we can feedback from all product owners who depend on it since they are aware of the potential implications on their end, and since it goes beyond the scope of Base UI, it's an overall unified library stack discussion.

Context

I saw this from the difference API used between number input and autocomplete in https://github.com/mui/base-ui/issues/579#issuecomment-2344004679.

Your environment

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Search keywords: -

atomiks commented 5 days ago

We went with onValueChange for a few reasons: