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

[EuiDatePickerRange] Applying onFocus/OnBlur handlers to inner EuiDatePicker components #6136

Merged
merged 3 commits into from
Aug 15, 2022
Merged

[EuiDatePickerRange] Applying onFocus/OnBlur handlers to inner EuiDatePicker components #6136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25524b3
Applying onFocus/OnBlur handlers to inner EuiDatePicker components.
anthonyhastings Aug 12, 2022
2b9381d
Adding upcoming changelog entry
anthonyhastings Aug 15, 2022
3ab1456
removing use of lodash from EuiDatePickerRange
anthonyhastings Aug 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion src/components/date_picker/date_picker_range.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import React from 'react';
import { render } from 'enzyme';
import { render, mount } from 'enzyme';
import { requiredProps } from '../../test';

import { EuiDatePickerRange } from './date_picker_range';
Expand Down Expand Up @@ -79,4 +79,51 @@ describe('EuiDatePickerRange', () => {

expect(component).toMatchSnapshot();
});

it('calls blur and focus handlers for date pickers while also triggering range control handlers', () => {
Copy link
Contributor

@cee-chen cee-chen Aug 12, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Thank you for the super explicit test!! Love it! Also agreed with making this one a unit test vs. a Cypress test, I think it's straightforward enough it doesn't need an E2E test.

Copy link
Contributor Author

@anthonyhastings anthonyhastings Aug 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Apologies, what I'd meant was that this repository currently has npm scripts that trigger cypress component testing (rather than E2E testing).

I was going to write the test as a cypress component test so it ran in isolation within a real browser, but the test would've been orphaned considering all the existing tests would've still been in enzyme. I thought it better to opt for co-location.

const rangeControlOnBlurMock = jest.fn();
const rangeControlOnFocusMock = jest.fn();
const startControlOnBlurMock = jest.fn();
const startControlOnFocusMock = jest.fn();
const endControlOnBlurMock = jest.fn();
const endControlOnFocusMock = jest.fn();

const component = mount(
<EuiDatePickerRange
onBlur={rangeControlOnBlurMock}
onFocus={rangeControlOnFocusMock}
startDateControl={
<EuiDatePicker
onBlur={startControlOnBlurMock}
onFocus={startControlOnFocusMock}
/>
}
endDateControl={
<EuiDatePicker
onBlur={endControlOnBlurMock}
onFocus={endControlOnFocusMock}
/>
}
/>
);

const startControl = component.find('EuiDatePicker').at(0);
const endControl = component.find('EuiDatePicker').at(1);

startControl.props().onFocus?.({} as React.FocusEvent);
expect(startControlOnFocusMock).toHaveBeenCalledTimes(1);
expect(rangeControlOnFocusMock).toHaveBeenCalledTimes(1);

startControl.props().onBlur?.({} as React.FocusEvent);
expect(startControlOnBlurMock).toHaveBeenCalledTimes(1);
expect(rangeControlOnBlurMock).toHaveBeenCalledTimes(1);

endControl.props().onFocus?.({} as React.FocusEvent);
expect(endControlOnFocusMock).toHaveBeenCalledTimes(1);
expect(rangeControlOnFocusMock).toHaveBeenCalledTimes(2);

endControl.props().onBlur?.({} as React.FocusEvent);
expect(endControlOnBlurMock).toHaveBeenCalledTimes(1);
expect(rangeControlOnBlurMock).toHaveBeenCalledTimes(2);
});
});
33 changes: 33 additions & 0 deletions src/components/date_picker/date_picker_range.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
*/

import React, {
FocusEvent,
FocusEventHandler,
Fragment,
FunctionComponent,
ReactNode,
Expand Down Expand Up @@ -60,7 +62,20 @@ export type EuiDatePickerRangeProps = CommonProps & {
*/
readOnly?: boolean;

/**
* Passes through to each control
*/
fullWidth?: boolean;

/**
* Triggered whenever the start or end controls are blurred
*/
onBlur?: FocusEventHandler<HTMLInputElement>;

/**
* Triggered whenever the start or end controls are focused
*/
onFocus?: FocusEventHandler<HTMLInputElement>;
};

export const EuiDatePickerRange: FunctionComponent<EuiDatePickerRangeProps> = ({
Expand All @@ -74,6 +89,8 @@ export const EuiDatePickerRange: FunctionComponent<EuiDatePickerRangeProps> = ({
readOnly,
isInvalid,
disabled,
onFocus,
onBlur,
...rest
}) => {
const classes = classNames(
Expand Down Expand Up @@ -104,6 +121,14 @@ export const EuiDatePickerRange: FunctionComponent<EuiDatePickerRangeProps> = ({
'euiDatePickerRange__start',
startDateControl.props.className
),
onBlur: (event: FocusEvent<HTMLInputElement>) => {
startDateControl.props?.onBlur?.(event);
onBlur?.(event);
},
onFocus: (event: FocusEvent<HTMLInputElement>) => {
startDateControl.props?.onFocus?.(event);
onFocus?.(event);
},
}
);

Expand All @@ -120,6 +145,14 @@ export const EuiDatePickerRange: FunctionComponent<EuiDatePickerRangeProps> = ({
'euiDatePickerRange__end',
endDateControl.props.className
),
onBlur: (event: FocusEvent<HTMLInputElement>) => {
endDateControl.props?.onBlur?.(event);
onBlur?.(event);
},
onFocus: (event: FocusEvent<HTMLInputElement>) => {
endDateControl.props?.onFocus?.(event);
onFocus?.(event);
},
}
);
}
Expand Down
4 changes: 4 additions & 0 deletions upcoming_changelogs/6136.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
**Bug fixes**

- Fixed `onBlur` and `onFocus` handlers from `EuiDatePickerRange` being incorrectly applied to wrapping element rather than the start/end control datepickers.