fix: Also apply sort params to existing fields in a ListGuesser #629
Conversation
| const fieldProps = { | ||
| ...field.props, | ||
| key: field.props.source + (orderField ? `-${orderField}` : ''), | ||
| sortBy: orderField, | ||
| sortable: !!orderField, | ||
| }; |
There was a problem hiding this comment.
I'm afraid this is overriding existing props on the field, e.g. if you had set sortable={false} then this is lost.
Besides, IMO this is not the react-admin way to do things.
As of late react-admin tends to avoid inspecting the children and overriding their props directly as this can lead to unexpected behaviors. Instead, I wonder if the best solution would rather be to update the getOverrideCode to include the sortBy and sortable props in the generated code, which would encourage users to keep those props once they move to providing their own children.
There was a problem hiding this comment.
I'm afraid this is overriding existing props on the field, e.g. if you had set
sortable={false}then this is lost.Besides, IMO this is not the react-admin way to do things. As of late react-admin tends to avoid inspecting the children and overriding their props directly as this can lead to unexpected behaviors. Instead, I wonder if the best solution would rather be to update the
getOverrideCodeto include thesortByandsortableprops in the generated code, which would encourage users to keep those props once they move to providing their own children.
thanks for the input, I will have a look.
Also apply sort params to existing fields in a ListGuesser instead of only "empty" listguessers.