-
Notifications
You must be signed in to change notification settings - Fork 842
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
[Inputs] Styles should prioritize disabled
over readOnly
#8239
Comments
👋 Thank you for your suggestion or request! While the EUI team agrees that it's valid, it's unlikely that we will prioritize this issue on our roadmap. We'll leave the issue open if you or anyone else in the community wants to implement it by contributing to EUI. If not, this issue will auto close in one year. |
We estimated this as a Large size effort because of the potential impact on Kibana and the potential to break a large number of FTR tests. |
@JasonStoltz That does not appear to be true. I made the changes in #8271 then built, packed and install them in kibana, see elastic/kibana#206372. The CI passed just fine, minus a few flaky tests. In general, I don't think tests assert the applied styles, rather they assert that the attribute is present. expect(getByText(/Click me/i).closest('button')).toBeDisabled(); |
After revisiting this issue, I think we should just go forward and prioritize @MichaelMarcialis just to make sure - are you fine with this change from the design point of view? |
If you mean "prioritize |
Yes prioritizes |
Is your feature request related to a problem? Please describe.
Related to #8238, a solution to this would be to set
readOnly
wheneverdisabled
is set, so something like this...The issue in this case is that the current styles prioritize
readOnly
whenever bothreadOnly
anddisabled
are applied together.See demo https://codesandbox.io/p/sandbox/adoring-christian-jk8t45
Describe the solution you'd like
The input styles prioritize
disabled
overreadOnly
. This is the same behavior as the basic html input.Describe alternatives you've considered
An alternate fix to #8238 would be to just check
disabled
before triggeringonChange
from a consumers perspective, but this is not ideal.Desired timeline
Unknown
cc: @MichaelMarcialis
The text was updated successfully, but these errors were encountered: