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

[EuiFormRow] Allow isDisabled to remove interaction from label #4987

Closed
cchaos opened this issue Jul 28, 2021 · 4 comments · Fixed by #5009
Closed

[EuiFormRow] Allow isDisabled to remove interaction from label #4987

cchaos opened this issue Jul 28, 2021 · 4 comments · Fixed by #5009

Comments

@cchaos
Copy link
Contributor

cchaos commented Jul 28, 2021

To elaborate the issue, here is a codepen of the above code with each variation.
(The "dont have permission to fetch" variation made no sense as I wouldn't've displayed the row at all, but I realised I forgot to add the "don't have permission to edit" variation).

I love the UX of labels focusing their inputs, but it doesn't make sense to any scenario other than enabled.

The existing PR was correct in that disabled form row's should disable their inputs (this is not my goal but is definitely relevant and is not a breaking change as disabled doesnt currently exist for form row) but it should also make the label appear like regular text.

And just to clarify & backtrack on my statement in the issue

If the label is disabled then it'd just render a normal EuiFormLabel, with no for attribute

The for attribute should still be present, but we should simply change the appearance to mimic a label without a for i.e., set cursor: default

Originally posted by @j-m in #4899 (comment)

@babs20
Copy link
Contributor

babs20 commented Jul 28, 2021

I can go ahead and create a pull request for this issue. I originally did something similar to this in #4908, but removed it due to it not being relevant to the original PR's purpose.

@babs20
Copy link
Contributor

babs20 commented Jul 29, 2021

Would this only be for EuiFormRow, or are there other form components that need this functionality as well?

@cchaos
Copy link
Contributor Author

cchaos commented Jul 29, 2021

Well the EuiFormLabel is going to need to accept an isDisabled prop now so that EuiFormRow can just pass it through. But those are the only two that would need to be touched to satisfy this PR.

@babs20
Copy link
Contributor

babs20 commented Jul 29, 2021

Sounds good, that was approach I was going to implement.

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

Successfully merging a pull request may close this issue.

2 participants