-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Upgrade Assistant] Fix filter deprecations search filter #57541
Conversation
Added validateRegExpString util
…her search filter behaviour, e.g. index management.
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
@jloleysens Inserting the callout below the search box throws it out of alignment with the controls on the right, which looks a little weird. I wonder if there's a solution to this that doesn't alter the layout? Maybe a red alert icon to the right of the search box, with a tooltip that explains the error in more detail? |
@cjcenizal I think to keep in line with something like Index Management We can just move the callout rendering location to below the controls? |
Good idea! |
The previous callout layout looked off-center next to the controls in the table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for fixing this and adding tests!
I left one nit about the text. I also had a couple comments around the code, but I see they relate to code that already existed, so feel free to ignore.
<EuiFlexItem grow={true}> | ||
<EuiFieldSearch | ||
isInvalid={filterInvalid} | ||
aria-label="Filter" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i18n
<EuiFieldSearch | ||
isInvalid={filterInvalid} | ||
aria-label="Filter" | ||
placeholder={intl.formatMessage({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you switch to i18n.translate()
here instead?
<EuiFlexGroup direction="column" responsive={false}> | ||
<EuiFlexItem grow={true}> | ||
<EuiFlexGroup alignItems="center" wrap={true} responsive={false}> | ||
<EuiFlexItem grow={true}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe true
is the default
title={i18n.translate( | ||
'xpack.upgradeAssistant.checkupTab.controls.filterErrorMessageLabel', | ||
{ | ||
defaultMessage: 'Filter Invalid: {searchTermError}', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this should probably be sentence case per https://elastic.github.io/eui/#/guidelines/writing.
Update "Filter Invalid:" to sentence case Remove inject intl wrapper from CheckupControls component Remove unnecessary grow={true}
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
) * Made eui search field not a controlled component Added validateRegExpString util * Update error message display. Use EuiCallOut and i18n to replicate other search filter behaviour, e.g. index management. * Remove unused variable * Update Jest snapshot * Updated layout for callout The previous callout layout looked off-center next to the controls in the table. * Update copy and remove intl Update "Filter Invalid:" to sentence case Remove inject intl wrapper from CheckupControls component Remove unnecessary grow={true} * Updated Jest component snapshot Co-authored-by: Elastic Machine <[email protected]>
) * Made eui search field not a controlled component Added validateRegExpString util * Update error message display. Use EuiCallOut and i18n to replicate other search filter behaviour, e.g. index management. * Remove unused variable * Update Jest snapshot * Updated layout for callout The previous callout layout looked off-center next to the controls in the table. * Update copy and remove intl Update "Filter Invalid:" to sentence case Remove inject intl wrapper from CheckupControls component Remove unnecessary grow={true} * Updated Jest component snapshot Co-authored-by: Elastic Machine <[email protected]>
…re/files-and-filetree * 'master' of github.com:elastic/kibana: (139 commits) Move Ace XJSON lexer-rules, worker and utils to es_ui_shared (elastic#57563) [Upgrade Assistant] Fix filter deprecations search filter (elastic#57541) [ML] New Platform server shim: update indices routes (elastic#57685) Bump redux dependencies (elastic#53348) [Index management] Client-side NP ready (elastic#57295) change id of x-pack event_log plugin to eventLog (elastic#57612) [eventLog] get kibana.index name from config instead of hard-coding it (elastic#57607) revert allow using any path to generate fixes ui titles (elastic#57535) Fix login redirect for expired sessions (elastic#57157) Expose Vis on the contract as it requires visTypes (elastic#56968) [SIEM][Detection Engine] Fixes queries to ignore errors when signals index is not present [Remote clusters] Migrate client-side code out of legacy (elastic#57365) Fix failed test reporter for SIEM Cypress use (elastic#57240) skip flaky suite (elastic#45244) update chromedriver to 80.0.1 (elastic#57602) change slack action to only report on whitelisted host name (elastic#57582) [kbn/optimizer] throw errors into stream on invalid completion (elastic#57735) moving visualize/utils to new platform (elastic#56650) ...
…57779) * Made eui search field not a controlled component Added validateRegExpString util * Update error message display. Use EuiCallOut and i18n to replicate other search filter behaviour, e.g. index management. * Remove unused variable * Update Jest snapshot * Updated layout for callout The previous callout layout looked off-center next to the controls in the table. * Update copy and remove intl Update "Filter Invalid:" to sentence case Remove inject intl wrapper from CheckupControls component Remove unnecessary grow={true} * Updated Jest component snapshot Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
…57780) * Made eui search field not a controlled component Added validateRegExpString util * Update error message display. Use EuiCallOut and i18n to replicate other search filter behaviour, e.g. index management. * Remove unused variable * Update Jest snapshot * Updated layout for callout The previous callout layout looked off-center next to the controls in the table. * Update copy and remove intl Update "Filter Invalid:" to sentence case Remove inject intl wrapper from CheckupControls component Remove unnecessary grow={true} * Updated Jest component snapshot Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
Summary
Fix #57054
Release Notes
Fixed an issue with filtering deprecations in Upgrade Assistant where invalid Regular Expressions would fail silently. The UI now reports the issue.
Screenshots
First iteration
Checklist
Delete any items that are not applicable to this PR.
For maintainers