diff --git a/packages/eui/changelogs/upcoming/9540.md b/packages/eui/changelogs/upcoming/9540.md new file mode 100644 index 000000000000..c6d1333020cc --- /dev/null +++ b/packages/eui/changelogs/upcoming/9540.md @@ -0,0 +1 @@ +- Added a warning when non-recommended units are used in `width`, `minWidth` or `maxWidth` props on ``, ``, `` as well as the `columns` configuration on `` and `` diff --git a/packages/eui/src/components/table/table_footer_cell.test.tsx b/packages/eui/src/components/table/table_footer_cell.test.tsx index d9ad3149c531..ce97e39a7788 100644 --- a/packages/eui/src/components/table/table_footer_cell.test.tsx +++ b/packages/eui/src/components/table/table_footer_cell.test.tsx @@ -18,6 +18,7 @@ import { WARNING_MESSAGE_MAX_WIDTH, WARNING_MESSAGE_MIN_WIDTH, WARNING_MESSAGE_WIDTH, + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT, } from './utils'; import type { EuiTableSharedWidthProps } from './types'; @@ -76,14 +77,14 @@ describe('EuiTableFooterCell', () => { it('accepts `style` prop', () => { const { getByRole } = renderInTableFooter( Test ); expect(getByRole('cell')).toHaveStyle({ - width: '20%', + width: '200px', minWidth: '123px', maxWidth: '456px', }); @@ -99,14 +100,14 @@ describe('EuiTableFooterCell', () => { it(`accepts \`${name}\` prop`, () => { const { getByRole } = renderInTableFooter( - + Test ); expect(getByRole('cell')).toHaveStyle({ ...defaultStyles, - [name]: '10%', + [name]: '100px', }); }); @@ -130,9 +131,9 @@ describe('EuiTableFooterCell', () => { console.warn = jest.fn(); const props = { - [name]: '10%', + [name]: '100px', style: { - [name]: '20%', + [name]: '200px', }, }; @@ -142,13 +143,28 @@ describe('EuiTableFooterCell', () => { expect(getByRole('cell')).toHaveStyle({ ...defaultStyles, - [name]: '10%', + [name]: '100px', }); expect(console.warn).toHaveBeenCalledWith(warningMessage); console.warn = originalConsoleWarn; }); + + it('warns when a not recommended unit is used', () => { + const originalConsoleWarn = console.warn; + console.warn = jest.fn(); + + renderInTableFooter( + Test + ); + + expect(console.warn).toHaveBeenCalledWith( + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT + ); + + console.warn = originalConsoleWarn; + }); }; describe('width', testProp('width', WARNING_MESSAGE_WIDTH)); diff --git a/packages/eui/src/components/table/table_header_cell.test.tsx b/packages/eui/src/components/table/table_header_cell.test.tsx index 08fcef7f2af9..05e0cae93ea3 100644 --- a/packages/eui/src/components/table/table_header_cell.test.tsx +++ b/packages/eui/src/components/table/table_header_cell.test.tsx @@ -16,6 +16,7 @@ import { WARNING_MESSAGE_MAX_WIDTH, WARNING_MESSAGE_MIN_WIDTH, WARNING_MESSAGE_WIDTH, + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT, } from './utils'; import { EuiTableIsResponsiveContext } from './mobile/responsive_context'; @@ -158,14 +159,14 @@ describe('EuiTableHeaderCell', () => { it('accepts `style` prop', () => { const { getByRole } = renderInTableHeader( Test ); expect(getByRole('columnheader')).toHaveStyle({ - width: '20%', + width: '200px', minWidth: '123px', maxWidth: '456px', }); @@ -181,14 +182,14 @@ describe('EuiTableHeaderCell', () => { it(`accepts \`${name}\` prop`, () => { const { getByRole } = renderInTableHeader( - + Test ); expect(getByRole('columnheader')).toHaveStyle({ ...defaultStyles, - [name]: '10%', + [name]: '100px', }); }); @@ -212,9 +213,9 @@ describe('EuiTableHeaderCell', () => { console.warn = jest.fn(); const props = { - [name]: '10%', + [name]: '100px', style: { - [name]: '20%', + [name]: '200px', }, }; @@ -224,13 +225,28 @@ describe('EuiTableHeaderCell', () => { expect(getByRole('columnheader')).toHaveStyle({ ...defaultStyles, - [name]: '10%', + [name]: '100px', }); expect(console.warn).toHaveBeenCalledWith(warningMessage); console.warn = originalConsoleWarn; }); + + it('warns when a not recommended unit is used', () => { + const originalConsoleWarn = console.warn; + console.warn = jest.fn(); + + renderInTableHeader( + Test + ); + + expect(console.warn).toHaveBeenCalledWith( + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT + ); + + console.warn = originalConsoleWarn; + }); }; describe('width', testProp('width', WARNING_MESSAGE_WIDTH)); diff --git a/packages/eui/src/components/table/table_header_cell_checkbox.test.tsx b/packages/eui/src/components/table/table_header_cell_checkbox.test.tsx index 821fd12f215a..99decf3c5848 100644 --- a/packages/eui/src/components/table/table_header_cell_checkbox.test.tsx +++ b/packages/eui/src/components/table/table_header_cell_checkbox.test.tsx @@ -15,6 +15,7 @@ import { WARNING_MESSAGE_MAX_WIDTH, WARNING_MESSAGE_MIN_WIDTH, WARNING_MESSAGE_WIDTH, + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT, } from './utils'; import type { EuiTableSharedWidthProps } from './types'; @@ -40,14 +41,14 @@ describe('EuiTableHeaderCellCheckbox', () => { it('accepts `style` prop', () => { const { getByRole } = renderInTableHeader( Test ); expect(getByRole('columnheader')).toHaveStyle({ - width: '20%', + width: '200px', minWidth: '123px', maxWidth: '456px', }); @@ -63,14 +64,14 @@ describe('EuiTableHeaderCellCheckbox', () => { it(`accepts \`${name}\` prop`, () => { const { getByRole } = renderInTableHeader( - + Test ); expect(getByRole('columnheader')).toHaveStyle({ ...defaultStyles, - [name]: '10%', + [name]: '100px', }); }); @@ -96,9 +97,9 @@ describe('EuiTableHeaderCellCheckbox', () => { console.warn = jest.fn(); const props = { - [name]: '10%', + [name]: '100px', style: { - [name]: '20%', + [name]: '200px', }, }; @@ -110,13 +111,30 @@ describe('EuiTableHeaderCellCheckbox', () => { expect(getByRole('columnheader')).toHaveStyle({ ...defaultStyles, - [name]: '10%', + [name]: '100px', }); expect(console.warn).toHaveBeenCalledWith(warningMessage); console.warn = originalConsoleWarn; }); + + it('warns when a not recommended unit is used', () => { + const originalConsoleWarn = console.warn; + console.warn = jest.fn(); + + renderInTableHeader( + + Test + + ); + + expect(console.warn).toHaveBeenCalledWith( + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT + ); + + console.warn = originalConsoleWarn; + }); }; describe('width', testProp('width', WARNING_MESSAGE_WIDTH)); diff --git a/packages/eui/src/components/table/table_row_cell.test.tsx b/packages/eui/src/components/table/table_row_cell.test.tsx index 0f64a8909012..213e3c2e3a9e 100644 --- a/packages/eui/src/components/table/table_row_cell.test.tsx +++ b/packages/eui/src/components/table/table_row_cell.test.tsx @@ -15,6 +15,7 @@ import { WARNING_MESSAGE_MAX_WIDTH, WARNING_MESSAGE_MIN_WIDTH, WARNING_MESSAGE_WIDTH, + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT, } from './utils'; import { EuiTableIsResponsiveContext } from './mobile/responsive_context'; @@ -202,14 +203,14 @@ describe('EuiTableRowCell', () => { it('accepts `style` prop', () => { const { getByRole } = renderInTableRow( Test ); expect(getByRole('cell')).toHaveStyle({ - width: '20%', + width: '200px', minWidth: '123px', maxWidth: '456px', }); @@ -225,12 +226,12 @@ describe('EuiTableRowCell', () => { it(`accepts \`${name}\` prop`, () => { const { getByRole } = renderInTableRow( - Test + Test ); expect(getByRole('cell')).toHaveStyle({ ...defaultStyles, - [name]: '10%', + [name]: '100px', }); }); @@ -254,9 +255,9 @@ describe('EuiTableRowCell', () => { console.warn = jest.fn(); const props = { - [name]: '10%', + [name]: '100px', style: { - [name]: '20%', + [name]: '200px', }, }; @@ -266,13 +267,28 @@ describe('EuiTableRowCell', () => { expect(getByRole('cell')).toHaveStyle({ ...defaultStyles, - [name]: '10%', + [name]: '100px', }); expect(console.warn).toHaveBeenCalledWith(warningMessage); console.warn = originalConsoleWarn; }); + + it('warns when a not recommended unit is used', () => { + const originalConsoleWarn = console.warn; + console.warn = jest.fn(); + + renderInTableRow( + Test + ); + + expect(console.warn).toHaveBeenCalledWith( + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT + ); + + console.warn = originalConsoleWarn; + }); }; describe('width', testProp('width', WARNING_MESSAGE_WIDTH)); diff --git a/packages/eui/src/components/table/utils.test.ts b/packages/eui/src/components/table/utils.test.ts index 635981dde505..d848a2b5c0da 100644 --- a/packages/eui/src/components/table/utils.test.ts +++ b/packages/eui/src/components/table/utils.test.ts @@ -11,26 +11,34 @@ import { WARNING_MESSAGE_MAX_WIDTH, WARNING_MESSAGE_MIN_WIDTH, WARNING_MESSAGE_WIDTH, + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT, } from './utils'; describe('resolveWidthPropsAsStyle', () => { + let originalConsoleWarn: typeof console.warn; + + beforeAll(() => { + originalConsoleWarn = console.warn; + console.warn = jest.fn(); + }); + + afterAll(() => { + console.warn = originalConsoleWarn; + }); + it('returns an empty style object when no style or width props are provided', () => { expect(resolveWidthPropsAsStyle(undefined, {})).toEqual({}); expect(resolveWidthPropsAsStyle({}, {})).toEqual({}); }); - const testPropAndStyle = (name: string, warningMessage: string) => () => { - let originalConsoleWarn: typeof console.warn; - - beforeAll(() => { - originalConsoleWarn = console.warn; - console.warn = jest.fn(); - }); - - afterAll(() => { - console.warn = originalConsoleWarn; - }); + it('warns when a not recommended unit is used', () => { + expect(resolveWidthPropsAsStyle({}, { width: '10%' })); + expect(console.warn).toHaveBeenCalledWith( + WARNING_MESSAGE_NOT_RECOMMENDED_UNIT + ); + }); + const testPropAndStyle = (name: string, warningMessage: string) => () => { it(`supports setting ${name} via the prop`, () => { expect(resolveWidthPropsAsStyle({}, { [name]: '123px' })).toEqual({ [name]: '123px', diff --git a/packages/eui/src/components/table/utils.ts b/packages/eui/src/components/table/utils.ts index 03efd934e96c..b6447bfcf147 100644 --- a/packages/eui/src/components/table/utils.ts +++ b/packages/eui/src/components/table/utils.ts @@ -27,6 +27,12 @@ export const WARNING_MESSAGE_MIN_WIDTH = export const WARNING_MESSAGE_MAX_WIDTH = 'Two `maxWidth` properties were provided. Provide only one of `style.maxWidth` or `maxWidth` to avoid conflicts.'; +/** + * @internal + */ +export const WARNING_MESSAGE_NOT_RECOMMENDED_UNIT = + 'Detected not recommended unit (%, vw, cqw, cqi) in cell width settings. Adjust the `width`, `minWidth` and `maxWidth` values to use absolute length units like `em` for text cells or `px` for static elements like icons or plots.'; + const normalizeValue = ( value: string | number | undefined ): string | undefined => { @@ -41,6 +47,18 @@ const normalizeValue = ( return value; }; +const UNIT_VALIDATOR_REGEX = /%|vw|cqw|cqi/; + +const shouldWarnAboutNotRecommendedUnit = ( + value: string | number | undefined +): boolean => { + if (typeof value === 'string') { + return UNIT_VALIDATOR_REGEX.test(value); + } + + return false; +}; + /** * @internal */ @@ -52,27 +70,40 @@ export const resolveWidthPropsAsStyle = ( maxWidth: rawMaxWidth, }: EuiTableSharedWidthProps ): CSSProperties => { - const width = normalizeValue(rawWidth); - const minWidth = normalizeValue(rawMinWidth); - const maxWidth = normalizeValue(rawMaxWidth); + const widthProp = normalizeValue(rawWidth); + const minWidthProp = normalizeValue(rawMinWidth); + const maxWidthProp = normalizeValue(rawMaxWidth); + const width = widthProp ?? style.width; + const minWidth = minWidthProp ?? style.minWidth; + const maxWidth = maxWidthProp ?? style.maxWidth; + + // Value validation block if (process.env.NODE_ENV !== 'production') { - if (style.width && width !== undefined) { + if (style.width && widthProp !== undefined) { console.warn(WARNING_MESSAGE_WIDTH); } - if (style.minWidth && minWidth !== undefined) { + if (style.minWidth && minWidthProp !== undefined) { console.warn(WARNING_MESSAGE_MIN_WIDTH); } - if (style.maxWidth && maxWidth !== undefined) { + if (style.maxWidth && maxWidthProp !== undefined) { console.warn(WARNING_MESSAGE_MAX_WIDTH); } + + if ( + shouldWarnAboutNotRecommendedUnit(width) || + shouldWarnAboutNotRecommendedUnit(minWidth) || + shouldWarnAboutNotRecommendedUnit(maxWidth) + ) { + console.warn(WARNING_MESSAGE_NOT_RECOMMENDED_UNIT); + } } return { - width: width || style.width, - minWidth: minWidth || style.minWidth, - maxWidth: maxWidth || style.maxWidth, + width, + minWidth, + maxWidth, }; };