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.

Sign up for GitHub

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

Jump to bottom

[EuiFieldText] Prevent onChange event on disabled inputs #8271

Merged
merged 5 commits into from
Jan 23, 2025
Merged

[EuiFieldText] Prevent onChange event on disabled inputs #8271

merged 5 commits into from
Jan 23, 2025
+19 −6

Conversation

nickofthyme
Copy link
Contributor

@nickofthyme nickofthyme commented Jan 13, 2025 •
edited
Loading

Summary

This alters the behavior of the EuiFieldText to prioritize disabled styling over readonly, the same as native HTML inputs.

Additionally, the disabled attribute does not prevent onChange events from firing. To counteract this behavior, we default the readonly attribute to the value of disabled unless it is explicitly set. With readonly set to true, the onChange event is prevented. Alternatively, we could conditionally call the onChange handler manually.

These changes were tested in kibana ftr in elastic/kibana#206372 with no failing tests.

Closes #8239

QA

General checklist

  • Release checklist
    • A changelog entry exists and is marked appropriately.

Sorry, something went wrong.

@nickofthyme
fix: fully disable inputs when disabled
Loading
Loading status checks…
4ffca69
@nickofthyme
Copy link
Contributor Author

Looking for feedback at this point, no rush to merge.

This was referenced Jan 13, 2025
Closed
Closed
@nickofthyme
Copy link
Contributor Author

@tkajtoch if you don't like syncing the readonly attribute with the value of disabled, I could change this to a functional prevention such as to only call the onChange when disabled is false. Lmk

@tkajtoch tkajtoch self-requested a review January 14, 2025 14:47
tkajtoch
tkajtoch reviewed Jan 20, 2025
View reviewed changes
Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with this change. Could you please add a changelog item and a JSDoc comment to readOnly prop to let users know it defaults to the value of disabled now?

@nickofthyme nickofthyme requested a review from tkajtoch January 21, 2025 17:26
@tkajtoch tkajtoch marked this pull request as ready for review January 21, 2025 18:22
@tkajtoch tkajtoch requested a review from a team as a code owner January 21, 2025 18:22
tkajtoch
tkajtoch approved these changes Jan 21, 2025
View reviewed changes
Copy link
Member

@tkajtoch tkajtoch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I'll merge it after today's EUI release

@nickofthyme
Remove changelog spelling fix
Loading
Loading status checks…
c018c1c
@nickofthyme
Merge branch 'main' into disable-inputs
Loading
Loading status checks…
efaca4f
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@nickofthyme nickofthyme merged commit ee4999d into elastic:main Jan 23, 2025
5 checks passed
@nickofthyme nickofthyme deleted the disable-inputs branch January 23, 2025 00:50
@tkajtoch tkajtoch mentioned this pull request Feb 5, 2025
Merged
weronikaolejniczak pushed a commit to cee-chen/eui that referenced this pull request Feb 10, 2025
@nickofthyme @weronikaolejniczak
[EuiFieldText] Prevent onChange event on disabled inputs (elastic#8271
27080fd 
)
delanni pushed a commit to elastic/kibana that referenced this pull request Feb 13, 2025
@tkajtoch @elasticmachine
Upgrade EUI to v99.2.0-borealis.0 (#209690)
Loading
Loading status checks…
e5958a4 
`99.1.0-borealis.0` ⏩ `99.2.0-borealis.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

---

## [`v99.2.0`](https://github.com/elastic/eui/releases/v99.2.0)

- Changed `EuiFieldText` styles to prioritize `disabled` styling over
`readonly`. ([#8271](elastic/eui#8271))
- Added `plugs` and `web` glyphs to `EuiIcon`
([#8285](elastic/eui#8285))
- Update `title` on `EuiColorPalettePickerPaletteProps` to be optional
([#8289](elastic/eui#8289))

**Bug fixes**

- Fixed an issue with EuiDataGrid with auto row height resulting in a
table of 0 height ([#8251](elastic/eui#8251))
- Fixed `disabled` behavior of `EuiFieldText` to prevent input changes.
([#8271](elastic/eui#8271))

**Borealis changes**

- [Visual Refresh] Provide hooks for color palette functions
([#8284](elastic/eui#8284))
- [Visual Refresh] Make Borealis default theme in EUI
([#8288](elastic/eui#8288))

---

As you can probably tell, there are plenty of updated tests and
snapshots this time around. They are the result of the following:

1. We've updated the default theme in `@elastic/eui` v99.2.0 to
Borealis, which led to kibana tests using `renderWithTheme`,
`mountWithTheme`, and other utility functions that wrap the tested
component in `<EuiThemeProvider>` to switch to the correct, new color
values. With this change, all tests should now use Borealis, regardless
of the testing framework or its configuration.
2. Because of the default theme change in EUI, some class name hash
fragments (e.g., `css-`**`wprskz`**`-euiButtonIcon-xs-empty-primary`)
regenerated.

Most Borealis-related changes in this PR are test env only and have no
impact on what users see when using Kibana.

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
JoseLuisGJ added a commit to JoseLuisGJ/kibana that referenced this pull request Feb 13, 2025
@JoseLuisGJ
Upgrade EUI to v99.2.0-borealis.0 (elastic#209690)
99e64b5 
`99.1.0-borealis.0` ⏩ `99.2.0-borealis.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

---

## [`v99.2.0`](https://github.com/elastic/eui/releases/v99.2.0)

- Changed `EuiFieldText` styles to prioritize `disabled` styling over
`readonly`. ([elastic#8271](elastic/eui#8271))
- Added `plugs` and `web` glyphs to `EuiIcon`
([elastic#8285](elastic/eui#8285))
- Update `title` on `EuiColorPalettePickerPaletteProps` to be optional
([elastic#8289](elastic/eui#8289))

**Bug fixes**

- Fixed an issue with EuiDataGrid with auto row height resulting in a
table of 0 height ([elastic#8251](elastic/eui#8251))
- Fixed `disabled` behavior of `EuiFieldText` to prevent input changes.
([elastic#8271](elastic/eui#8271))

**Borealis changes**

- [Visual Refresh] Provide hooks for color palette functions
([elastic#8284](elastic/eui#8284))
- [Visual Refresh] Make Borealis default theme in EUI
([elastic#8288](elastic/eui#8288))

---

As you can probably tell, there are plenty of updated tests and
snapshots this time around. They are the result of the following:

1. We've updated the default theme in `@elastic/eui` v99.2.0 to
Borealis, which led to kibana tests using `renderWithTheme`,
`mountWithTheme`, and other utility functions that wrap the tested
component in `<EuiThemeProvider>` to switch to the correct, new color
values. With this change, all tests should now use Borealis, regardless
of the testing framework or its configuration.
2. Because of the default theme change in EUI, some class name hash
fragments (e.g., `css-`**`wprskz`**`-euiButtonIcon-xs-empty-primary`)
regenerated.

Most Borealis-related changes in this PR are test env only and have no
impact on what users see when using Kibana.

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

## Summary

Summarize your PR. If it involves visual changes include a screenshot or gif.

### Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md)
- [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials
- [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
- [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations.
- [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed
- [ ] The PR  description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

### Identify risks

Does this PR introduce any risks? For example, consider risks like hard to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] [See some risk examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
- [ ] ...
@JoseLuisGJ JoseLuisGJ mentioned this pull request Feb 13, 2025
Closed
9 tasks
tkajtoch added a commit to tkajtoch/kibana that referenced this pull request Feb 13, 2025
@tkajtoch
Upgrade EUI to v99.2.0-borealis.0 (elastic#209690)
1aa724c 
`99.1.0-borealis.0` ⏩ `99.2.0-borealis.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

---

## [`v99.2.0`](https://github.com/elastic/eui/releases/v99.2.0)

- Changed `EuiFieldText` styles to prioritize `disabled` styling over
`readonly`. ([elastic#8271](elastic/eui#8271))
- Added `plugs` and `web` glyphs to `EuiIcon`
([elastic#8285](elastic/eui#8285))
- Update `title` on `EuiColorPalettePickerPaletteProps` to be optional
([elastic#8289](elastic/eui#8289))

**Bug fixes**

- Fixed an issue with EuiDataGrid with auto row height resulting in a
table of 0 height ([elastic#8251](elastic/eui#8251))
- Fixed `disabled` behavior of `EuiFieldText` to prevent input changes.
([elastic#8271](elastic/eui#8271))

**Borealis changes**

- [Visual Refresh] Provide hooks for color palette functions
([elastic#8284](elastic/eui#8284))
- [Visual Refresh] Make Borealis default theme in EUI
([elastic#8288](elastic/eui#8288))

---

As you can probably tell, there are plenty of updated tests and
snapshots this time around. They are the result of the following:

1. We've updated the default theme in `@elastic/eui` v99.2.0 to
Borealis, which led to kibana tests using `renderWithTheme`,
`mountWithTheme`, and other utility functions that wrap the tested
component in `<EuiThemeProvider>` to switch to the correct, new color
values. With this change, all tests should now use Borealis, regardless
of the testing framework or its configuration.
2. Because of the default theme change in EUI, some class name hash
fragments (e.g., `css-`**`wprskz`**`-euiButtonIcon-xs-empty-primary`)
regenerated.

Most Borealis-related changes in this PR are test env only and have no
impact on what users see when using Kibana.

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit e5958a4)

# Conflicts:
#	src/platform/packages/shared/kbn-unified-data-table/src/components/compare_documents/hooks/__snapshots__/use_comparison_css.test.ts.snap
#	x-pack/solutions/security/packages/kbn-securitysolution-exception-list-components/src/exception_item_card/conditions/entry_content/__snapshots__/entry_content.test.tsx.snap
#	x-pack/solutions/security/plugins/security_solution/public/explore/network/components/embeddables/__mocks__/mock.ts
@tkajtoch tkajtoch mentioned this pull request Feb 20, 2025
Merged
mgadewoll added a commit to elastic/kibana that referenced this pull request Feb 27, 2025
@tkajtoch @elasticmachine
@acstll @kibanamachine @mgadewoll
[8.x] Upgrade EUI to v99.3.0-classic.0 (#211917)
Loading
Loading status checks…
25ee7a5 
`98.2.2-borealis.0` ⏩ `99.3.0-classic.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

---

## [`v99.3.0`](https://github.com/elastic/eui/releases/v99.3.0)

- Updated table components to support adding tooltips to header cells
([#8273](elastic/eui#8273))
  - Added `columns.nameTooltip` on `EuiBasicTable`
  - Added `tooltipProps` prop on `EuiTableHeaderCell`
- Added `accent` color option to `EuiCallOut`
([#8303](elastic/eui#8303))
- Updated `EuiInlineEditForm`'s `onCancel` prop type to allow
uncontrolled mode usage
([#8307](elastic/eui#8307))
- Added ES|QL syntax highlighting for `EuiCode`, `EuiCodeBlock`,
`EuiMarkdownFormat`, and `EuiMarkdownEditor` components.
([#8317](elastic/eui#8317))
- Updated `EuiAccordion` to prevent content from being transitioned on
initial render when `initialIsOpen=true`
([#8327](elastic/eui#8327))

**Bug fixes**

- Fixed a bug on `EuiSuperDatePicker` where pasting an absolute date
would append the date instead of replace it
([#8311](elastic/eui#8311))

## [`v99.2.0`](https://github.com/elastic/eui/releases/v99.2.0)

- Changed `EuiFieldText` styles to prioritize `disabled` styling over
`readonly`. ([#8271](elastic/eui#8271))
- Added `plugs` and `web` glyphs to `EuiIcon`
([#8285](elastic/eui#8285))
- Update `title` on `EuiColorPalettePickerPaletteProps` to be optional
([#8289](elastic/eui#8289))

**Bug fixes**

- Fixed an issue with EuiDataGrid with auto row height resulting in a
table of 0 height ([#8251](elastic/eui#8251))
- Fixed `disabled` behavior of `EuiFieldText` to prevent input changes.
([#8271](elastic/eui#8271))

## [`v99.1.0`](https://github.com/elastic/eui/releases/v99.1.0)

- Updated `EuiColorPalettePicker` - adds `append` to
`EuiColorPalettePickerPaletteProps` to support appending custom content
to the title ([#8208](elastic/eui#8208))
- Updated font-weight and font-size of `EuiBetaBadge`s to improve
legibility ([#8255](elastic/eui#8255))
- Added support for `titleColor` variant `warning` on `EuiStat`
([#8278](elastic/eui#8278))

## [`v99.0.0`](https://github.com/elastic/eui/releases/v99.0.0)

- Added two new icons: `createGenericJob` and `createGeoJob`
([#8248](elastic/eui#8248))
- Added new icon `section`
([#8261](elastic/eui#8261))

**Bug fixes**

- Ensures that the `values` of `EuiI18n` used in `EuiPagination` use
`key` attributes to prevent potential ["unique key"
warnings](https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-key).
([#8243](elastic/eui#8243))

**Breaking changes**

- Removed `EuiPopover`'s deprecated `hasDragDrop` prop. Use `usePortal`
on any child `EuiDraggable` instead
([#8256](elastic/eui#8256))

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Arturo Castillo Delgado <arturo.castillo@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Lene Gadewoll <lene.gadewoll@elastic.co>
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Mar 4, 2025
@tkajtoch @elasticmachine
@acstll @kibanamachine @mgadewoll @SoniaSanzV
[8.x] Upgrade EUI to v99.3.0-classic.0 (elastic#211917)
f725d82 
`98.2.2-borealis.0` ⏩ `99.3.0-classic.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

---

## [`v99.3.0`](https://github.com/elastic/eui/releases/v99.3.0)

- Updated table components to support adding tooltips to header cells
([elastic#8273](elastic/eui#8273))
  - Added `columns.nameTooltip` on `EuiBasicTable`
  - Added `tooltipProps` prop on `EuiTableHeaderCell`
- Added `accent` color option to `EuiCallOut`
([elastic#8303](elastic/eui#8303))
- Updated `EuiInlineEditForm`'s `onCancel` prop type to allow
uncontrolled mode usage
([elastic#8307](elastic/eui#8307))
- Added ES|QL syntax highlighting for `EuiCode`, `EuiCodeBlock`,
`EuiMarkdownFormat`, and `EuiMarkdownEditor` components.
([elastic#8317](elastic/eui#8317))
- Updated `EuiAccordion` to prevent content from being transitioned on
initial render when `initialIsOpen=true`
([elastic#8327](elastic/eui#8327))

**Bug fixes**

- Fixed a bug on `EuiSuperDatePicker` where pasting an absolute date
would append the date instead of replace it
([elastic#8311](elastic/eui#8311))

## [`v99.2.0`](https://github.com/elastic/eui/releases/v99.2.0)

- Changed `EuiFieldText` styles to prioritize `disabled` styling over
`readonly`. ([elastic#8271](elastic/eui#8271))
- Added `plugs` and `web` glyphs to `EuiIcon`
([elastic#8285](elastic/eui#8285))
- Update `title` on `EuiColorPalettePickerPaletteProps` to be optional
([elastic#8289](elastic/eui#8289))

**Bug fixes**

- Fixed an issue with EuiDataGrid with auto row height resulting in a
table of 0 height ([elastic#8251](elastic/eui#8251))
- Fixed `disabled` behavior of `EuiFieldText` to prevent input changes.
([elastic#8271](elastic/eui#8271))

## [`v99.1.0`](https://github.com/elastic/eui/releases/v99.1.0)

- Updated `EuiColorPalettePicker` - adds `append` to
`EuiColorPalettePickerPaletteProps` to support appending custom content
to the title ([elastic#8208](elastic/eui#8208))
- Updated font-weight and font-size of `EuiBetaBadge`s to improve
legibility ([elastic#8255](elastic/eui#8255))
- Added support for `titleColor` variant `warning` on `EuiStat`
([elastic#8278](elastic/eui#8278))

## [`v99.0.0`](https://github.com/elastic/eui/releases/v99.0.0)

- Added two new icons: `createGenericJob` and `createGeoJob`
([elastic#8248](elastic/eui#8248))
- Added new icon `section`
([elastic#8261](elastic/eui#8261))

**Bug fixes**

- Ensures that the `values` of `EuiI18n` used in `EuiPagination` use
`key` attributes to prevent potential ["unique key"
warnings](https://react.dev/learn/rendering-lists#keeping-list-items-in-order-with-key).
([elastic#8243](elastic/eui#8243))

**Breaking changes**

- Removed `EuiPopover`'s deprecated `hasDragDrop` prop. Use `usePortal`
on any child `EuiDraggable` instead
([elastic#8256](elastic/eui#8256))

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Arturo Castillo Delgado <arturo.castillo@elastic.co>
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Lene Gadewoll <lene.gadewoll@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkajtoch tkajtoch

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Inputs] Styles should prioritize disabled over readOnly
4 participants
@nickofthyme @kibanamachine @elasticmachine @tkajtoch