Skip to content
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

BooleanField no longer displays label by default #6549

Closed
flouc001 opened this issue Sep 2, 2021 · 6 comments · Fixed by #6553
Closed

BooleanField no longer displays label by default #6549

flouc001 opened this issue Sep 2, 2021 · 6 comments · Fixed by #6553

Comments

@flouc001
Copy link

flouc001 commented Sep 2, 2021

What you were expecting:
When using the BooleanField the label should be displayed by default due to the defaultProps setting addLabel to true.

What happened instead:
BooleanField no longer displays the label without explicitly specifying the addLabel prop, this is breaking.

Steps to reproduce:

  1. Add a BooleanField component to a view with the label prop set.
  2. Observe the field being rendered without the label.
  3. Add the addLabel prop.
  4. Observe the label being rendered.

Related code:
https://codesandbox.io/s/eloquent-silence-1te8b

Observe the tick with no label on this page: https://1te8b.sse.codesandbox.io/#/posts/13/show/2

Other information:
I think it's down to the memo being moved directly to the default export in this commit: b79787a#diff-d7c4dd12e78cb28e0370dc53068aa45ec475f6fe54b5b40db238547e6278f5aaR119

It's no longer respecting the defaultProps.

Environment

  • React-admin version: 3.17.3
  • Last version that did not exhibit the issue (if applicable): 3.17.2
  • React version: 17
  • Browser: Chrome
  • Stack trace (in case of a JS error): N/A
@flouc001 flouc001 changed the title BooleanInputField no longer displays label by default BooleanField no longer displays label by default Sep 2, 2021
@flouc001
Copy link
Author

flouc001 commented Sep 2, 2021

@WiXSL what's the motivation to moving away from FC typing? I'm guessing you moved the memo because Typescript was throwing an error.

@WiXSL
Copy link
Contributor

WiXSL commented Sep 2, 2021

@WiXSL what's the motivation to moving away from FC typing? I'm guessing you moved the memo because Typescript was throwing an error.

Here: #6515 (comment)

One solution could be to use default values for props inside the component instead of BooleanField.defaultProps

@flouc001
Copy link
Author

flouc001 commented Sep 2, 2021

@WiXSL thank you for the response - the motivation makes sense! I raised a PR with an idea for maybe moving the responsibility to the displaying components. Let me know what you think, just throwing some ideas out there.

@WiXSL
Copy link
Contributor

WiXSL commented Sep 2, 2021

We have to ping @fzaninotto and @djhi on how to solve this, but if this is a problem for addLabel is a problem for all defaultProps of Field components defined using propTypes

@flouc001
Copy link
Author

flouc001 commented Sep 2, 2021

Agreed, that's why I was thinking of moving the logic to the component with the responsibility of displaying the component. That way it fixes all of the fields that may encounter this issue rather than just BooleanField.

@eamahanna
Copy link

I also encountered this issue with the DateField and ArrayField.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants