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

[EuiSuperSelect] Ignores readOnly prop #3510

Closed
mwswindle opened this issue May 22, 2020 · 4 comments · Fixed by #5738
Closed

[EuiSuperSelect] Ignores readOnly prop #3510

mwswindle opened this issue May 22, 2020 · 4 comments · Fixed by #5738

Comments

@mwswindle
Copy link

mwswindle commented May 22, 2020

I'm developing a plugin on Kibana v7.7.0 tag, which is using elastic/eui v21.0.1.

I'm using the EuiSuperSelect component, which has a readOnly prop. If I set this prop to true, I can see the styling of the component change, removing the shadows, but I'm still able to change the value.

I have several other EUI components (such as EuiFieldText) that use the same readOnlyState variable, that all behave as expected.

<EuiSuperSelect
  readOnly={readOnlyState} // <-- Changes style, but not function
  valueOfSelected={itemId}
  onChange={setItemId} // <-- This is called regardless of *readOnlyState*
  options={itemOptions}
/>

image

@cchaos
Copy link
Contributor

cchaos commented May 26, 2020

Hi @mwswindle! I noticed this too pretty recently, thank you for making an issue.

The problem is that <select> HTML elements that this component is mimicking doesn't actually support readonly so it's actually more of a mistake to both

a. support the prop
b. apply the styles for it

It's more appropriate to use the disabled prop. Though it does beg the question whether or not we should "fake" a readonly state for our own components.

@cchaos cchaos mentioned this issue May 27, 2020
11 tasks
@cchaos cchaos changed the title EuiSuperSelect ignoring readOnly prop [EuiSuperSelect] Ignores readOnly prop Sep 20, 2020
@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@cchaos
Copy link
Contributor

cchaos commented Sep 17, 2021

I think we should go ahead and support the readOnly prop in functionality as well as look. This means:

  • Removing any interaction behavior when the prop is true
  • Removing the down arrow when the prop is true.

@cchaos cchaos self-assigned this Mar 22, 2022
cchaos pushed a commit to cchaos/eui that referenced this issue Mar 22, 2022
- Fixed EuiSuperSelect border radius with append/prepend elastic#5442
- Fixed EuiSuperSelect not respecting `readOnly` elastic#3510
cchaos added a commit that referenced this issue Mar 30, 2022
* [EuiFormControlLayout] Added `isInvalid` prop which is passed through to `EuiFormControlLayoutIcon` which renders the `alert` icon in red

* Passed the `isInvalid` through for some form controls
- EuiFieldNumber, EuiFieldPassword, EuiFieldText, EuiSelect, EuiSuperSelect
- EuiFieldSearch attempts to create a new class for the number of icons

* Added a `getFormControlClassNameForIconCount` localized service for couting icons

* Change className output from rendering component-specific to generic to reduce output syles

* [EuiFormControlLayout] Added `isDropdown` to create and control the arrow down icon

* [EuiColorPicker] Fix usage of EuiFormControlLayout to not double

* [EuiSelect] & [EuiSuperSelect] Update to new EuiFormControlLayout props
- Fixed EuiSuperSelect border radius with append/prepend #5442
- Fixed EuiSuperSelect not respecting `readOnly` #3510

* [EuiValidatableControl] Add `aria-invalid`
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