Skip to content

Commit

Permalink
fix failing test case
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari committed Feb 8, 2021
1 parent a15b07d commit e0d155c
Show file tree
Hide file tree
Showing 12 changed files with 32 additions and 41 deletions.
5 changes: 1 addition & 4 deletions docs/src/pages/components/slider/ContinuousSlider.tsx
Expand Up @@ -9,10 +9,7 @@ import VolumeUp from '@material-ui/icons/VolumeUp';
export default function ContinuousSlider() {
const [value, setValue] = React.useState<number>(30);

const handleChange = (
event: React.SyntheticEvent,
newValue: number | number[],
) => {
const handleChange = (event: unknown, newValue: number | number[]) => {
setValue(newValue as number);
};

Expand Down
5 changes: 1 addition & 4 deletions docs/src/pages/components/slider/NonLinearSlider.tsx
Expand Up @@ -24,10 +24,7 @@ function calculateValue(value: number) {
export default function NonLinearSlider() {
const [value, setValue] = React.useState<number>(10);

const handleChange = (
event: React.SyntheticEvent,
newValue: number | number[],
) => {
const handleChange = (event: unknown, newValue: number | number[]) => {
if (typeof newValue === 'number') {
setValue(newValue);
}
Expand Down
5 changes: 1 addition & 4 deletions docs/src/pages/components/slider/RangeSlider.tsx
Expand Up @@ -10,10 +10,7 @@ function valuetext(value: number) {
export default function RangeSlider() {
const [value, setValue] = React.useState<number[]>([20, 37]);

const handleChange = (
event: React.SyntheticEvent,
newValue: number | number[],
) => {
const handleChange = (event: unknown, newValue: number | number[]) => {
setValue(newValue as number[]);
};

Expand Down
2 changes: 1 addition & 1 deletion docs/translations/api-docs/select/select.json
Expand Up @@ -16,7 +16,7 @@
"MenuProps": "Props applied to the <a href=\"/api/menu/\"><code>Menu</code></a> element.",
"multiple": "If <code>true</code>, <code>value</code> must be an array and the menu will support multiple selections.",
"native": "If <code>true</code>, the component uses a native <code>select</code> element.",
"onChange": "Callback fired when a menu item is selected.<br><br><strong>Signature:</strong><br><code>function(event: object, child?: object) =&gt; void</code><br><em>event:</em> The event source of the callback. You can pull out the new value by accessing <code>event.target.value</code> (any).<br><em>child:</em> The react element that was selected when <code>native</code> is <code>false</code> (default).",
"onChange": "Callback fired when a menu item is selected.<br><br><strong>Signature:</strong><br><code>function(event: object, child?: object) =&gt; void</code><br><em>event:</em> The event source of the callback. You can pull out the new value by accessing <code>event.target.value</code> (any). <strong>Warning</strong>: This is a generic event not a change event.<br><em>child:</em> The react element that was selected when <code>native</code> is <code>false</code> (default).",
"onClose": "Callback fired when the component requests to be closed. Use in controlled mode (see open).<br><br><strong>Signature:</strong><br><code>function(event: object) =&gt; void</code><br><em>event:</em> The event source of the callback.",
"onOpen": "Callback fired when the component requests to be opened. Use in controlled mode (see open).<br><br><strong>Signature:</strong><br><code>function(event: object) =&gt; void</code><br><em>event:</em> The event source of the callback.",
"open": "If <code>true</code>, the component is shown. You can only use it when the <code>native</code> prop is <code>false</code> (default).",
Expand Down
Expand Up @@ -173,17 +173,19 @@ export interface SliderUnstyledTypeMap<P = {}, D extends React.ElementType = 'sp
/**
* Callback function that is fired when the slider's value changed.
*
* @param {object} event The event source of the callback. **Warning**: This is a generic event not a change event.
* @param {object} event The event source of the callback.
* You can pull out the new value by accessing `event.target.value` (any).
* **Warning**: This is a generic event not a change event.
* @param {number | number[]} value The new value.
*/
onChange?: (event: React.SyntheticEvent, value: number | number[]) => void;
onChange?: (event: Event, value: number | number[]) => void;
/**
* Callback function that is fired when the `mouseup` is triggered.
*
* @param {object} event The event source of the callback. **Warning**: This is a generic event not a change event.
* @param {number | number[]} value The new value.
*/
onChangeCommitted?: (event: React.SyntheticEvent, value: number | number[]) => void;
onChangeCommitted?: (event: React.SyntheticEvent | Event, value: number | number[]) => void;
/**
* The component orientation.
* @default 'horizontal'
Expand Down
15 changes: 4 additions & 11 deletions packages/material-ui-unstyled/src/SliderUnstyled/SliderUnstyled.js
Expand Up @@ -230,23 +230,16 @@ const SliderUnstyled = React.forwardRef(function SliderUnstyled(props, ref) {
const handleChange =
onChange &&
((event, value) => {
if (!(event instanceof Event)) event.persist();

// Redefine target to allow name and value to be read.
// This allows seamless integration with the most popular form libraries.
// https://github.com/mui-org/material-ui/issues/13485#issuecomment-676048492
let clonedEvent;
// Clone the event to not override `target` of the native event.
// No need to worry about it for SyntheticEvent as they are systematically cloned.
if (event instanceof Event) {
clonedEvent = new event.constructor(event.type, event);
} else {
clonedEvent = event;
}
// Clone the event to not override `target` of the original event.
const nativeEvent = event.nativeEvent || event;
const clonedEvent = new nativeEvent.constructor(nativeEvent.type, nativeEvent);

Object.defineProperty(clonedEvent, 'target', {
writable: true,
value: { value, name, target: event.target },
value: { value, name, event },
});

onChange(clonedEvent, value);
Expand Down
1 change: 1 addition & 0 deletions packages/material-ui/src/Select/Select.d.ts
Expand Up @@ -113,6 +113,7 @@ export interface SelectProps<T = unknown>
*
* @param {object} event The event source of the callback.
* You can pull out the new value by accessing `event.target.value` (any).
* **Warning**: This is a generic event not a change event.
* @param {object} [child] The react element that was selected when `native` is `false` (default).
*/
onChange?: SelectInputProps<T>['onChange'];
Expand Down
1 change: 1 addition & 0 deletions packages/material-ui/src/Select/Select.js
Expand Up @@ -184,6 +184,7 @@ Select.propTypes = {
*
* @param {object} event The event source of the callback.
* You can pull out the new value by accessing `event.target.value` (any).
* **Warning**: This is a generic event not a change event.
* @param {object} [child] The react element that was selected when `native` is `false` (default).
*/
onChange: PropTypes.func,
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Select/SelectInput.d.ts
Expand Up @@ -15,7 +15,7 @@ export interface SelectInputProps<T = unknown> {
native: boolean;
onBlur?: React.FocusEventHandler<any>;
onChange?: (
event: React.ChangeEvent<{ name?: string; value: T }>,
event: React.ChangeEvent<{ name?: string; value: T; event: Event | React.SyntheticEvent }>,
child: React.ReactNode
) => void;
onClose?: (event: React.SyntheticEvent) => void;
Expand Down
15 changes: 10 additions & 5 deletions packages/material-ui/src/Select/SelectInput.js
Expand Up @@ -189,13 +189,18 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
setValueState(newValue);

if (onChange) {
event.persist();
// Preact support, target is read only property on a native event.
Object.defineProperty(event, 'target', {
// Redefine target to allow name and value to be read.
// This allows seamless integration with the most popular form libraries.
// https://github.com/mui-org/material-ui/issues/13485#issuecomment-676048492
// Clone the event to not override `target` of the original event.
const nativeEvent = event.nativeEvent || event;
const clonedEvent = new nativeEvent.constructor(nativeEvent.type, nativeEvent);

Object.defineProperty(clonedEvent, 'target', {
writable: true,
value: { value: newValue, name },
value: { value: newValue, name, event },
});
onChange(event, child);
onChange(clonedEvent, child);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/Slider/Slider.spec.tsx
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react';
import Slider from '@material-ui/core/Slider';

function testOnChange() {
function handleSliderChange(event: React.SyntheticEvent, tabsValue: unknown) {}
function handleSliderChange(event: Event, value: unknown) {}
<Slider onChange={handleSliderChange} onChangeCommitted={handleSliderChange} />;

function handleElementChange(event: React.ChangeEvent) {}
Expand Down
12 changes: 5 additions & 7 deletions packages/material-ui/src/Slider/Slider.test.js
Expand Up @@ -950,11 +950,10 @@ describe('<Slider />', () => {
});

expect(handleChange.callCount).to.equal(1);
expect(handleChange.firstCall.returnValue).to.deep.equal({
name: 'change-testing',
target: slider,
value: 4,
});
const returnValue = handleChange.firstCall.returnValue;
expect(returnValue.name).to.equal('change-testing');
expect(returnValue.value).to.equal(4);
expect(returnValue.event).to.not.equal(undefined);
});

describe('prop: ValueLabelComponent', () => {
Expand Down Expand Up @@ -1004,8 +1003,7 @@ describe('<Slider />', () => {
expect(handleEvent.returnValues).to.have.members([slider]);
});

// eslint-disable-next-line mocha/no-skipped-tests -- FIXME
it.skip('should not override the event.target on mouse events', () => {
it('should not override the event.target on mouse events', () => {
const handleNativeEvent = spy((event) => event.target);
const handleEvent = spy((event) => event.target);
function Test() {
Expand Down

0 comments on commit e0d155c

Please sign in to comment.