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

[EuiTextArea] Add isClearable and icon props #7449

Merged
merged 4 commits into from
Jan 5, 2024

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Jan 5, 2024

Summary

Added text area clearable and icon options , motivation is to replace this custom implementation in widely used kibana component https://github.com/elastic/kibana/blob/main/src/plugins/unified_search/public/query_string_input/query_string_input.tsx#L860

when invalid and isClearable both props are provided, both icons overlaps each other.

QA

textarea_clear

General checklist

  • Browser QA
    - [ ] Checked in mobile
    • Checked in both light and dark modes
    • Checked in Chrome, Safari, Edge, and Firefox
    • Checked for accessibility including keyboard-only and screenreader modes
  • Docs site QA
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
      - [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist
    • [ ] Updated the Figma library counterpart Looks like we can skip this, as the Figma designs for the Search field don't contain a clearable icon either

@shahzad31 shahzad31 requested a review from a team as a code owner January 5, 2024 11:20
@shahzad31 shahzad31 changed the title feat: text area clearable and icon options [EuiTextArea]text area clearable and icon options Jan 5, 2024
* Shows a button that quickly clears any input
*/
isClearable?: boolean;
icon?: EuiFormControlLayoutIconsProps['icon'];
Copy link
Member

Choose a reason for hiding this comment

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

Not a blocking question, just me being curious - what was the impetus behind adding icon as well? Is it needed in Kibana also, or was it mostly just for parity with EuiFieldText?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's also needed in kibana

+ props reorder
+ tests - significantly easier to test in Cypress/E2E than Jest/jsdom

+ bonus perf memoization
@cee-chen cee-chen changed the title [EuiTextArea]text area clearable and icon options [EuiTextArea]Add isClearable and icon props Jan 5, 2024
@cee-chen
Copy link
Member

cee-chen commented Jan 5, 2024

@shahzad31 Alrighty, logic should be all fixed now. My only remaining question for you before merge is about the icon prop, just to make there there's some intentionality there when we add it!

@cee-chen cee-chen changed the title [EuiTextArea]Add isClearable and icon props [EuiTextArea] Add isClearable and icon props Jan 5, 2024
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@cee-chen cee-chen merged commit 02e14a4 into elastic:main Jan 5, 2024
7 checks passed
@shahzad31 shahzad31 deleted the text-area-clearable branch January 6, 2024 01:14
cee-chen added a commit to elastic/kibana that referenced this pull request Jan 10, 2024
`v91.3.1`⏩`v92.0.0-backport.0`

---

##
[`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0)

**This is a backport release only intended for use by Kibana.**

**Bug fixes**

- Fixed an `EuiTreeView` JSX Typescript error
([#7452](elastic/eui#7452))
- Fixed a color console warning being generated by disabled `EuiStep`s
([#7454](elastic/eui#7454))


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

- Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and
`EuiSearchBar.Query.execute` to add `extends object` constraint
([#7340](elastic/eui#7340))
- This change should have no impact on your applications since the
updated types only affect properties that exclusively accept object
values.
- Added a new `EuiFlyoutResizable` component
([#7439](elastic/eui#7439))
- Updated `EuiTextArea` to accept `isClearable` and `icon` as props
([#7449](elastic/eui#7449))

**Bug fixes**

- `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their
positions on resize ([#7442](elastic/eui#7442))

**Deprecations**

- Updated `EuiFilterButton` to remove the second
`.euiFilterButton__textShift` span wrapper. Target
`.euiFilterButton__text` instead
([#7444](elastic/eui#7444))

**Breaking changes**

- Removed deprecated `EuiNotificationEvent`. We recommend copying the
component to your application if necessary
([#7434](elastic/eui#7434))
- Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar`
instead ([#7435](elastic/eui#7435))
delanni pushed a commit to delanni/kibana that referenced this pull request Jan 11, 2024
`v91.3.1`⏩`v92.0.0-backport.0`

---

##
[`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0)

**This is a backport release only intended for use by Kibana.**

**Bug fixes**

- Fixed an `EuiTreeView` JSX Typescript error
([elastic#7452](elastic/eui#7452))
- Fixed a color console warning being generated by disabled `EuiStep`s
([elastic#7454](elastic/eui#7454))


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

- Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and
`EuiSearchBar.Query.execute` to add `extends object` constraint
([elastic#7340](elastic/eui#7340))
- This change should have no impact on your applications since the
updated types only affect properties that exclusively accept object
values.
- Added a new `EuiFlyoutResizable` component
([elastic#7439](elastic/eui#7439))
- Updated `EuiTextArea` to accept `isClearable` and `icon` as props
([elastic#7449](elastic/eui#7449))

**Bug fixes**

- `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their
positions on resize ([elastic#7442](elastic/eui#7442))

**Deprecations**

- Updated `EuiFilterButton` to remove the second
`.euiFilterButton__textShift` span wrapper. Target
`.euiFilterButton__text` instead
([elastic#7444](elastic/eui#7444))

**Breaking changes**

- Removed deprecated `EuiNotificationEvent`. We recommend copying the
component to your application if necessary
([elastic#7434](elastic/eui#7434))
- Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar`
instead ([elastic#7435](elastic/eui#7435))
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
`v91.3.1`⏩`v92.0.0-backport.0`

---

##
[`v92.0.0-backport.0`](https://github.com/elastic/eui/releases/v92.0.0-backport.0)

**This is a backport release only intended for use by Kibana.**

**Bug fixes**

- Fixed an `EuiTreeView` JSX Typescript error
([elastic#7452](elastic/eui#7452))
- Fixed a color console warning being generated by disabled `EuiStep`s
([elastic#7454](elastic/eui#7454))


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

- Updated generic types of `EuiBasicTable`, `EuiInMemoryTable` and
`EuiSearchBar.Query.execute` to add `extends object` constraint
([elastic#7340](elastic/eui#7340))
- This change should have no impact on your applications since the
updated types only affect properties that exclusively accept object
values.
- Added a new `EuiFlyoutResizable` component
([elastic#7439](elastic/eui#7439))
- Updated `EuiTextArea` to accept `isClearable` and `icon` as props
([elastic#7449](elastic/eui#7449))

**Bug fixes**

- `EuiRange`/`EuiDualRange`'s track ticks & highlights now update their
positions on resize ([elastic#7442](elastic/eui#7442))

**Deprecations**

- Updated `EuiFilterButton` to remove the second
`.euiFilterButton__textShift` span wrapper. Target
`.euiFilterButton__text` instead
([elastic#7444](elastic/eui#7444))

**Breaking changes**

- Removed deprecated `EuiNotificationEvent`. We recommend copying the
component to your application if necessary
([elastic#7434](elastic/eui#7434))
- Removed deprecated `EuiControlBar`. We recommend using `EuiBottomBar`
instead ([elastic#7435](elastic/eui#7435))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants