feat(slider): Migrate to custom slider due to range input limitations (…

…#1410) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
box · Jul 16, 2021 · 19722e6 · 19722e6
1 parent e1d5280
commit 19722e6
Show file tree

Hide file tree

Showing 14 changed files with 388 additions and 247 deletions.
diff --git a/src/lib/Preview.js b/src/lib/Preview.js
@@ -1876,7 +1876,7 @@ class Preview extends EventEmitter {
         // Ignore key events when we are inside certain fields
         if (
             !target ||
-            (KEYDOWN_EXCEPTIONS.indexOf(target.nodeName) > -1 && !target.getAttribute('data-allow-keydown')) ||
+            KEYDOWN_EXCEPTIONS.indexOf(target.nodeName) > -1 ||
             (target.nodeName === 'DIV' && !!target.getAttribute('contenteditable'))
         ) {
             return;

diff --git a/src/lib/__tests__/Preview-test.js b/src/lib/__tests__/Preview-test.js
@@ -2762,15 +2762,6 @@ describe('lib/Preview', () => {
             expect(stubs.event.preventDefault).not.toBeCalled();
         });
 
-        test('should pass the event to the viewer if the target is an input but allows keydown handling', () => {
-            stubs.decodeKeydown.mockReturnValue('M');
-            stubs.event.target.getAttribute = jest.fn(() => true); // data-allow-keydown=true
-            preview.viewer.onKeydown = jest.fn(() => true);
-            preview.keydownHandler(stubs.event);
-
-            expect(preview.viewer.onKeydown).toBeCalledWith('M', stubs.event);
-        });
-
         test('should navigate left is key is ArrowLeft and the event has not been consumed', () => {
             preview.viewer.onKeydown = jest.fn(() => false);
             stubs.event.target.nodeName = 'ArrowLeft';

diff --git a/src/lib/viewers/controls/media/TimeControls.scss b/src/lib/viewers/controls/media/TimeControls.scss
@@ -1,35 +1,31 @@
-@import '../slider/styles';
-
-$bp-TimeControls-size: 18px; // Divisible by 3px (default) and 6px (hovered) for better vertical centering
-$bp-TimeControls-space: 10px;
+@import '../styles';
 
 .bp-TimeControls {
-    height: $bp-TimeControls-size;
+    position: relative;
+    margin-right: 10px;
+    margin-left: 10px;
+}
 
-    .bp-SliderControl-input {
-        padding-right: $bp-TimeControls-space;
-        padding-left: $bp-TimeControls-space;
+.bp-TimeControls-slider {
+    height: 18px; // Divisible by 3px (default) and 6px (hovered) for better vertical centering
 
-        @include bp-SliderThumb {
-            transform: scale(0);
-            transition: transform 100ms ease;
-            will-change: transform; // Prevent flickering in Safari
-        }
+    .bp-SliderControl-thumb {
+        transform: scale(0);
+        transition: transform 100ms ease;
+        will-change: transform; // Prevent flickering in Safari
     }
 
     .bp-SliderControl-track {
-        margin-right: $bp-TimeControls-space;
-        margin-left: $bp-TimeControls-space;
         backface-visibility: hidden; // Prevent jank in Firefox
         transition: transform 100ms ease;
         will-change: transform; // Prevent flickering in Safari
     }
 
+    &.bp-is-scrubbing,
+    &:focus,
     &:hover {
-        .bp-SliderControl-input {
-            @include bp-SliderThumb {
-                transform: scale(1);
-            }
+        .bp-SliderControl-thumb {
+            transform: scale(1);
         }
 
         .bp-SliderControl-track {

diff --git a/src/lib/viewers/controls/media/TimeControls.tsx b/src/lib/viewers/controls/media/TimeControls.tsx
@@ -25,22 +25,24 @@ export default function TimeControls({
     durationTime = 0,
     onTimeChange,
 }: Props): JSX.Element {
-    const currentValue = isFinite(currentTime) && isFinite(durationTime) ? percent(currentTime, durationTime) : 0;
+    const currentValue = isFinite(currentTime) ? currentTime : 0;
+    const durationValue = isFinite(durationTime) ? durationTime : 0;
+    const currentPercentage = percent(currentValue, durationValue);
     const bufferedAmount = bufferedRange && bufferedRange.length ? bufferedRange.end(bufferedRange.length - 1) : 0;
-    const bufferedValue = percent(bufferedAmount, durationTime);
-
-    const handleChange = (newValue: number): void => {
-        onTimeChange(round(durationTime * (newValue / 100)));
-    };
+    const bufferedPercentage = percent(bufferedAmount, durationValue);
 
     return (
-        <SliderControl
-            className="bp-TimeControls"
-            onChange={handleChange}
-            step={0.1}
-            title={__('media_time_slider')}
-            track={`linear-gradient(to right, ${bdlBoxBlue} ${currentValue}%, ${white} ${currentValue}%, ${white} ${bufferedValue}%, ${bdlGray62} ${bufferedValue}%, ${bdlGray62} 100%)`}
-            value={currentValue}
-        />
+        <div className="bp-TimeControls">
+            <SliderControl
+                className="bp-TimeControls-slider"
+                max={durationValue}
+                min={0}
+                onUpdate={onTimeChange}
+                step={5}
+                title={__('media_time_slider')}
+                track={`linear-gradient(to right, ${bdlBoxBlue} ${currentPercentage}%, ${white} ${currentPercentage}%, ${white} ${bufferedPercentage}%, ${bdlGray62} ${bufferedPercentage}%, ${bdlGray62} 100%)`}
+                value={currentValue}
+            />
+        </div>
     );
 }
diff --git a/src/lib/viewers/controls/media/VolumeControls.scss b/src/lib/viewers/controls/media/VolumeControls.scss
@@ -18,7 +18,10 @@
 }
 
 .bp-VolumeControls-slider {
-    width: 100px;
+    width: 100px - $bp-SliderControl-thumb-size;
+    height: 100%;
+    margin-right: $bp-SliderControl-thumb-radius;
+    margin-left: $bp-SliderControl-thumb-radius;
 }
 
 .bp-VolumeControls-toggle {

diff --git a/src/lib/viewers/controls/media/VolumeControls.tsx b/src/lib/viewers/controls/media/VolumeControls.tsx
@@ -8,7 +8,6 @@ import IconVolumeMute24 from '../icons/IconVolumeMute24';
 import MediaToggle from './MediaToggle';
 import SliderControl from '../slider';
 import useAttention from '../hooks/useAttention';
-import { decodeKeydown } from '../../../util';
 import './VolumeControls.scss';
 
 export type Props = {
@@ -38,34 +37,30 @@ export default function VolumeControls({ onMuteChange, onVolumeChange, volume =
     const title = isMuted ? __('media_unmute') : __('media_mute');
     const value = Math.round(volume * 100);
 
-    const handleKeydown = (event: React.KeyboardEvent): void => {
-        const key = decodeKeydown(event);
-
-        // Allow the range input to handle its own left/right keydown events
-        if (key === 'ArrowLeft' || key === 'ArrowRight') {
-            event.stopPropagation(); // Prevents global key handling
-        }
-    };
-
-    const handleMute = (): void => {
-        onMuteChange(!isMuted);
-    };
-
-    const handleVolume = (newValue: number): void => {
-        onVolumeChange(newValue / 100);
-    };
+    const handleVolume = React.useCallback(
+        (newValue: number): void => {
+            onVolumeChange(newValue / 100);
+        },
+        [onVolumeChange],
+    );
 
     return (
         <div className="bp-VolumeControls">
-            <MediaToggle className="bp-VolumeControls-toggle" onClick={handleMute} title={title} {...handlers}>
+            <MediaToggle
+                className="bp-VolumeControls-toggle"
+                onClick={(): void => onMuteChange(!isMuted)}
+                title={title}
+                {...handlers}
+            >
                 <Icon />
             </MediaToggle>
 
             <div className={classNames('bp-VolumeControls-flyout', { 'bp-is-open': isActive })}>
                 <SliderControl
                     className="bp-VolumeControls-slider"
-                    onChange={handleVolume}
-                    onKeyDown={handleKeydown}
+                    max={100}
+                    onUpdate={handleVolume}
+                    step={1}
                     title={__('media_volume_slider')}
                     track={`linear-gradient(to right, ${bdlBoxBlue} ${value}%, ${white} ${value}%)`}
                     value={value}

diff --git a/src/lib/viewers/controls/media/__tests__/TimeControls-test.tsx b/src/lib/viewers/controls/media/__tests__/TimeControls-test.tsx
@@ -12,61 +12,40 @@ describe('TimeControls', () => {
     const getWrapper = (props = {}): ShallowWrapper =>
         shallow(<TimeControls currentTime={0} durationTime={10000} onTimeChange={jest.fn()} {...props} />);
 
-    describe('event handlers', () => {
-        test.each`
-            percentage | value
-            ${0}       | ${0}
-            ${20}      | ${1111}
-            ${33}      | ${1833.15}
-            ${33.3333} | ${1851.6648}
-            ${100}     | ${5555}
-        `(
-            'should calculate the absolute value $value based on the relative slider percentage $percentage',
-            ({ percentage, value }) => {
-                const onChange = jest.fn();
-                const wrapper = getWrapper({ durationTime: 5555, onTimeChange: onChange });
-                const slider = wrapper.find(SliderControl);
-
-                slider.simulate('change', percentage);
-
-                expect(onChange).toBeCalledWith(value);
-            },
-        );
-    });
-
     describe('render', () => {
         test('should return a valid wrapper', () => {
             const wrapper = getWrapper();
 
             expect(wrapper.hasClass('bp-TimeControls')).toBe(true);
-            expect(wrapper.prop('step')).toEqual(0.1);
+            expect(wrapper.find(SliderControl).props()).toMatchObject({
+                max: 10000,
+                min: 0,
+                step: 5,
+                value: 0,
+            });
         });
 
-        test('should not calculate percentage for invalid currentTime value', () => {
-            const wrapper = getWrapper({ currentTime: NaN, durationTime: 100 });
+        test('should not default to zero for invalid currentTime value', () => {
+            const wrapper = getWrapper({ currentTime: NaN });
             expect(wrapper.find(SliderControl).prop('value')).toBe(0);
         });
 
-        test('should not calculate percentage for invalid durationTime value', () => {
-            const wrapper = getWrapper({ currentTime: 100, durationTime: NaN });
-            expect(wrapper.find(SliderControl).prop('value')).toBe(0);
+        test('should default to zero for invalid durationTime value', () => {
+            const wrapper = getWrapper({ durationTime: NaN });
+            expect(wrapper.find(SliderControl).prop('max')).toBe(0);
         });
 
         test.each`
-            currentTime | track                                                                                        | value
-            ${0}        | ${'linear-gradient(to right, #0061d5 0%, #fff 0%, #fff 10%, #767676 10%, #767676 100%)'}     | ${0}
-            ${50}       | ${'linear-gradient(to right, #0061d5 0.5%, #fff 0.5%, #fff 10%, #767676 10%, #767676 100%)'} | ${0.5}
-            ${1000}     | ${'linear-gradient(to right, #0061d5 10%, #fff 10%, #fff 10%, #767676 10%, #767676 100%)'}   | ${10}
-            ${2500}     | ${'linear-gradient(to right, #0061d5 25%, #fff 25%, #fff 10%, #767676 10%, #767676 100%)'}   | ${25}
-            ${10000}    | ${'linear-gradient(to right, #0061d5 100%, #fff 100%, #fff 10%, #767676 10%, #767676 100%)'} | ${100}
-        `('should render the correct track and value for currentTime $currentTime', ({ currentTime, track, value }) => {
+            currentTime | track
+            ${0}        | ${'linear-gradient(to right, #0061d5 0%, #fff 0%, #fff 10%, #767676 10%, #767676 100%)'}
+            ${50}       | ${'linear-gradient(to right, #0061d5 0.5%, #fff 0.5%, #fff 10%, #767676 10%, #767676 100%)'}
+            ${1000}     | ${'linear-gradient(to right, #0061d5 10%, #fff 10%, #fff 10%, #767676 10%, #767676 100%)'}
+            ${2500}     | ${'linear-gradient(to right, #0061d5 25%, #fff 25%, #fff 10%, #767676 10%, #767676 100%)'}
+            ${10000}    | ${'linear-gradient(to right, #0061d5 100%, #fff 100%, #fff 10%, #767676 10%, #767676 100%)'}
+        `('should render the correct track for currentTime $currentTime', ({ currentTime, track }) => {
             const buffer = getBuffer(1000, 0); // 10% buffered
             const wrapper = getWrapper({ bufferedRange: buffer, currentTime });
-
-            expect(wrapper.find(SliderControl).props()).toMatchObject({
-                track,
-                value,
-            });
+            expect(wrapper.find(SliderControl).prop('track')).toEqual(track);
         });
     });
 });
diff --git a/src/lib/viewers/controls/media/__tests__/VolumeControls-test.tsx b/src/lib/viewers/controls/media/__tests__/VolumeControls-test.tsx
@@ -28,15 +28,6 @@ describe('VolumeControls', () => {
             toggle.simulate('click');
             expect(onMuteChange).toBeCalledWith(isMuted);
         });
-
-        test.each(['ArrowLeft', 'ArrowRight'])('should defer to the inner input for the %s key', key => {
-            const event = { key, stopPropagation: jest.fn() };
-            const wrapper = getWrapper();
-
-            wrapper.find(SliderControl).simulate('keydown', event);
-
-            expect(event.stopPropagation).toBeCalled();
-        });
     });
 
     describe('render', () => {

diff --git a/src/lib/viewers/controls/media/_styles.scss b/src/lib/viewers/controls/media/_styles.scss
@@ -1,3 +1,4 @@
+@import '../slider/variables';
 @import '../styles';
 
 $bp-MediaControl-height: 40px;