add onChanged prop on Slider (#6108)

[blandine]

Pull request checklist

• Addresses an existing issue: Fixes Slider onChange event should be triggered when the mouse click has been released #6108
• Include a change request file using $ npm run change

Description of changes

In Slider component, there is a new prop 'onBlur' which is called on mouseUp or touchEnd

Focus areas to test

I tried to add test in the .spec file but I don't know how to simulate the mouseUp event since it's a listener on the window object. I'm open to suggestions :)

Microsoft Reviewers: Open in CodeFlow

[msftclas]

All CLA requirements met.

[micahgodbolt]

Mouse or touch, right?

[blandine]

Yes indeed, I'll change the comment.

[micahgodbolt]

We've had a request to make a small change to the prop.

1. onBlur should be reserved for the actual onBlur event on the button (mouse release is actually blurring)
2. as we eventually need to change onChange to onChange(event, item) (A breaking change) lets be sure that our new prop uses this new, non deprecated interface

So lets go with onChanged (past tense) with an interface of (ev, value). This will take a few tweaks up in _onMouseDownOrTouchStart to pass the event through.

[micahgodbolt]

Appreciate the changes @blandine, once these changes are in I'll get this merged and a build will kick off before the next business day.

[blandine]

Thanks for the review, I'll change it today (hopefully!).

[micahgodbolt]

Does this work without modifying lines 167 and 170? Maybe i just don't know how that _events.on callback takes in props.

Last thing then, can you add a quick test to make sure this onChanged returns the event and correct number?

[blandine]

I tried to add a test previously but I didn't succeed to simulate a mousedown and then a mouseup to trigger the onChanged.
I followed this issue enzymejs/enzyme#426 and this one : enzymejs/enzyme#1357 saying that we should not use Enzyme simulate anymore.
Could you give me some pointers?
Thanks

[micahgodbolt]

For now, i'd just piggyback off of the existing onChange test and make sure your onChanged callback is being fired as well (including event). I agree, it seems enzyme is moving away from simulate, so no need to do anything elaborate.

[blandine]

It can't work since onChanged is fired only when the mouseup or touchend happens.
On mousedown event (like in the extisting onChange test), onChanged won't be triggered.
I try to hack something like in the callout.test.tsx but it doesn't work :

const mouseEvent = document.createEvent('MouseEvent');
    const root = document.createElement('div');
    document.body.appendChild(root);

    ReactDOM.render<HTMLDivElement>(<Slider onChanged={onChanged} />, root);

    mouseEvent.initMouseEvent('mousedown', false, false, window, 0, 0, 0, 0, 0, false, false, false, false, 1, root);
    mouseEvent.initMouseEvent('mouseup', false, false, window, 0, 0, 0, 0, 0, false, false, false, false, 1, root);

Thank you for your time but I'm afraid I won't be able to complete this PR :/

[micahgodbolt]

Last change, just add a quick test, then this is good to go.

[micahgodbolt]

I don't want this to block your ability to contribute. Thanks for trying as hard as you could with the tests.