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

Cannot disable form inputs #759

Closed
adrians5j opened this issue Mar 17, 2020 · 8 comments · Fixed by #775
Closed

Cannot disable form inputs #759

adrians5j opened this issue Mar 17, 2020 · 8 comments · Fixed by #775
Labels
bug good first issue An issue that is not too complicated to resolve, and it's an ideal issue for new contributors.

Comments

@adrians5j
Copy link
Member

adrians5j commented Mar 17, 2020

This is:

  • Bug

Detailed Description

Passing a disabled={true} prop on components like Input and Select does not have any effect. For example:

https://storybook.webiny.com/?path=/story/components-input--usage
https://storybook.webiny.com/?path=/story/components-select--usage

It works in the RMWC library - https://rmwc.io/text-fields.

Not sure where the bug is but if I hardcoded the disabled={true} in packages/ui/src/Input/Input.tsx:112, all inputs would become disabled as expected:

image

So, it seems that the flag is not correctly passed to the actual RMWC TextArea component.

@adrians5j adrians5j added bug good first issue An issue that is not too complicated to resolve, and it's an ideal issue for new contributors. labels Mar 17, 2020
@ahmad-reza619
Copy link
Contributor

hey @doitadrian i've seen the Input component, is it possible for missing prop types cause this? i think the implementation is already good

@Pavel910
Copy link
Collaborator

@ahmad-reza619 what do you mean by missing prop types?

@ahmad-reza619
Copy link
Contributor

@Pavel910 the Input component has no disabled prop types, but it use disabled prop, so I think it automatically remove props that doesn't specified in prop type. It's just my guess actually, I never used typescript before

@Pavel910
Copy link
Collaborator

Actually that’s not correct, as you can see we compose types by importing the original Rmwc type and add our own props on top of that.

Also, TS doesn’t remove anything, no tools do. You can’t remove a prop from component; whatever is passed to component will be in the props object at runtime.

This is an interesting bug, I’ll take a look at it as well.

@adrians5j
Copy link
Member Author

I found the issue here. Actually, it's related to the Bind component and how it spreads props to the child component:

packages/form/src/Bind.ts:78:
image

Changing this to the following fixed the issue:

image

@ahmad-reza619
Copy link
Contributor

Is it already fixed?

@Pavel910
Copy link
Collaborator

@ahmad-reza619 nope, feel free to submit a PR if you'd like to take on this :)

@ahmad-reza619
Copy link
Contributor

Alright will do, but wow, i never thought another component is the cause of all this 🤔 how is it even possible? is Bind forward it props to the children?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue An issue that is not too complicated to resolve, and it's an ideal issue for new contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants