From 07bd76c2a3a85cb4b1d18ae44eda56f04d4a40aa Mon Sep 17 00:00:00 2001 From: Constance Date: Thu, 8 Jul 2021 08:58:59 -0700 Subject: [PATCH] [App Search] Result Settings: Max size input fixes (#104755) * Fix "Max size" number inputs cutting off the "No limit" placeholder text on Chrome/webkit - textOnly={false} is required for the input to expand in width * Fix handleFieldNumberBlur behavior: - should allow setting a number field back to empty/no limits - dev note: '' is the blank/empty state, which gets parsed to NaN. Random invalid strings ALSO get parsed to '', which is NaN * Fix input not clearing invalid non-number strings For some really odd reason, '' and undefined aren't correctly resetting the input value when a string gets entered into the input. I had to add a space for EUI to actually start clearing it Note that this change completely prevents users from writing non-numbers into the input at all, but this is probably fine/valid honestly --- .../result_settings_table/field_number.test.tsx | 6 +++--- .../result_settings_table/field_number.tsx | 11 +++++------ .../result_settings_table/text_fields_body.tsx | 4 ++-- .../result_settings_table/text_fields_header.tsx | 4 ++-- 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/field_number.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/field_number.test.tsx index 3ac50d906e9c..c012167f6781 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/field_number.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/field_number.test.tsx @@ -54,7 +54,7 @@ describe('FieldNumber', () => { }} /> ); - expect(wrapper.find(EuiFieldNumber).prop('value')).toEqual(''); + expect(wrapper.find(EuiFieldNumber).prop('value')).toEqual(' '); }); it('is disabled if the [fieldEnabledProperty] in fieldSettings is false', () => { @@ -90,10 +90,10 @@ describe('FieldNumber', () => { expect(props.updateAction).toHaveBeenCalledWith('foo', 21); }); - it('will call updateAction on blur using the minimum possible value if the current value is something other than a number', () => { + it('will call clearAction on blur if the current value is something other than a number', () => { const wrapper = shallow(); wrapper.simulate('blur', { target: { value: '' } }); - expect(props.updateAction).toHaveBeenCalledWith('foo', 20); + expect(props.clearAction).toHaveBeenCalledWith('foo'); }); it('will call updateAction on blur using the minimum possible value if the value is something lower than the minimum', () => { diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/field_number.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/field_number.tsx index cd7bab3c6f59..f16bab5234ab 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/field_number.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/field_number.tsx @@ -43,11 +43,10 @@ const handleFieldNumberBlur = ( clearAction: (fieldName: string) => void ) => { return (e: FocusEvent) => { - const value = parseInt(e.target.value, 10); - const fieldValue = Math.min( - SIZE_FIELD_MAXIMUM, - Math.max(SIZE_FIELD_MINIMUM, isNaN(value) ? 0 : value) - ); + let fieldValue = parseInt(e.target.value, 10); + if (!isNaN(fieldValue)) { + fieldValue = Math.min(SIZE_FIELD_MAXIMUM, Math.max(SIZE_FIELD_MINIMUM, fieldValue)); + } updateOrClearSizeForField(fieldName, fieldValue, updateAction, clearAction); }; }; @@ -74,7 +73,7 @@ export const FieldNumber: React.FC = ({ value={ typeof fieldSettings[fieldSizeProperty] === 'number' ? (fieldSettings[fieldSizeProperty] as number) - : '' + : ' ' // Without the space, invalid non-numbers don't get cleared for some reason } placeholder={i18n.translate( 'xpack.enterpriseSearch.appSearch.engine.resultSettings.numberFieldPlaceholder', diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/text_fields_body.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/text_fields_body.tsx index 3a2eb20fecdf..c3b46f585272 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/text_fields_body.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/result_settings/result_settings_table/text_fields_body.tsx @@ -56,7 +56,7 @@ export const TextFieldsBody: React.FC = () => { }} /> - + { }} /> - + { { defaultMessage: 'Raw' } )} - + {i18n.translate( 'xpack.enterpriseSearch.appSearch.engine.resultSettings.table.column.maxSizeTitle', { defaultMessage: 'Max size' } @@ -48,7 +48,7 @@ export const TextFieldsHeader: React.FC = () => { { defaultMessage: 'Fallback' } )} - + {i18n.translate( 'xpack.enterpriseSearch.appSearch.engine.resultSettings.table.column.maxSizeTitle', { defaultMessage: 'Max size' }