-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Add a default unique id for useInput
#9788
Conversation
@@ -132,7 +132,7 @@ export const useInput = <ValueType = any>( | |||
}; | |||
|
|||
return { | |||
id: id || finalSource, | |||
id: id || `use-input-${finalSource}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should leverage useId
if id
is not provided
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
applied
id: id || finalSource, | ||
id: | ||
id || | ||
`${defaultId.substring(1, defaultId.length - 1)}-${finalSource}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just defaultId
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was to be more readable.
Applied
docs/useInput.md
Outdated
| `format` | Optional | `Function` | - | A function to format the value from the record to the input value | | ||
| `parse` | Optional | `Function` | - | A function to parse the value from the input to the record value | | ||
| `validate` | Optional | `Function` | `Function[]` | - | A function or an array of functions to validate the input value | | ||
| `id` | Optional | `string` | `:r[input number]:` | The id of the input | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too much details here. Just use autogenerated
for the default value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
id: id || finalSource, | ||
id: | ||
id || | ||
`${defaultId.substring(1, defaultId.length - 1)}-${finalSource}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you substring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
You should probably add a section in the upgrade guide as this is a breaking change |
## Inputs default ids are auto-generated | ||
|
||
In previous versions, the input default id was the source of the input. In v5, inputs defaults ids are auto-generated with [React useId()](https://react.dev/reference/react/useId). | ||
|
||
**Tip:** You still can pass an id as prop of any [react-admin input](./Inputs.md) or use a [reference](https://fr.react.dev/reference/react/useRef). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the impact for users? You should mention that if they relied on this for tests, they should pass the id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
something like this ?
To do
Closes #9534