feat(ui): more efficient sortable list component with drag'n drop events

feugy · Oct 1, 2020 · f98684d · f98684d
1 parent 7d08213
commit f98684d
Show file tree

Hide file tree

Showing 15 changed files with 331 additions and 325 deletions.
diff --git a/README.md b/README.md
@@ -53,7 +53,6 @@ There are thunsands of them in the wild. This mine is an excuse for learning [El
 
 ### Bugs and known issues
 
-1. now that I'm not using FLIP animations, native drag'n drop may be leaner than my mouse-event based implementation
 1. Undetected live changes: remove tracks and re-add them. This is a linux-only issue with chokidar
    - https://github.com/paulmillr/chokidar/issues/917
    - https://github.com/paulmillr/chokidar/issues/591

diff --git a/renderer/App.svelte b/renderer/App.svelte
@@ -8,7 +8,6 @@
     Nav,
     Sheet,
     Snackbar,
-    SortableListConstants,
     SystemNotifier,
     TracksQueue,
     Tutorial
@@ -23,7 +22,6 @@
   let isPlaylistOpen = true
   let ready = false
   let scrollable
-  const { isMoveInProgress } = SortableListConstants
 
   onMount(async () => {
     const settings = await invoke('settings.get')
@@ -84,17 +82,11 @@
   <div>
     <main>
       <Sheet bind:open={isPlaylistOpen}>
-        <section
-          slot="main"
-          bind:this={scrollable}
-          use:autoScrollable={{ enabled: $isMoveInProgress }}>
+        <section slot="main" bind:this={scrollable} use:autoScrollable>
           <Nav />
           <Router {scrollable} />
         </section>
-        <aside
-          slot="aside"
-          id="queue"
-          use:autoScrollable={{ enabled: $isMoveInProgress }}>
+        <aside slot="aside" id="queue" use:autoScrollable>
           <TracksQueue />
         </aside>
       </Sheet>

diff --git a/renderer/actions/auto-scrollable/__snapshots__/auto-scrollable.stories.storyshot b/renderer/actions/auto-scrollable/__snapshots__/auto-scrollable.stories.storyshot
@@ -4,98 +4,164 @@ exports[`Storyshots Actions/auto-scrollable Default 1`] = `
 <section
   class="storybook-snapshot-container"
 >
+  <p
+    class="p-4 font-bold"
+  >
+    Caution: does not work on Firefox (DragEvent always have 0 clientY)
+  </p>
+
   <ol
     class="svelte-886hq0"
     style="--padding: 75px; --margin: -75px;"
   >
-    <li>
+    <li
+      draggable="true"
+    >
       Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       In eu mi bibendum neque egestas congue quisque egestas
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Arcu non odio euismod lacinia at quis risus sed
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Massa tempor nec feugiat nisl
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Sagittis aliquam malesuada bibendum arcu vitae elementum
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Elementum facilisis leo vel fringilla est ullamcorper eget nulla
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Bibendum arcu vitae elementum curabitur vitae nunc sed velit dignissim
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Elit sed vulputate mi sit amet mauris commodo
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Ut enim blandit volutpat maecenas volutpat blandit aliquam etiam erat
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Lorem mollis aliquam ut porttitor leo a diam sollicitudin
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Vitae proin sagittis nisl rhoncus mattis
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Ut morbi tincidunt augue interdum velit euismod in pellentesque
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Aliquam purus sit amet luctus venenatis lectus magna
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Amet venenatis urna cursus eget
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Et pharetra pharetra massa massa
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Ac ut consequat semper viverra nam libero justo laoreet
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Eget arcu dictum varius duis at consectetur lorem
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Tellus id interdum velit laoreet id donec
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Aliquam sem fringilla ut morbi
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Consequat nisl vel pretium lectus quam id leo in vitae
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Velit scelerisque in dictum non
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Pretium aenean pharetra magna ac
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Tincidunt arcu non sodales neque sodales ut etiam
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Cursus eget nunc scelerisque viverra mauris in aliquam
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Tortor at auctor urna nunc id cursus
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Consequat interdum varius sit amet mattis vulputate enim nulla
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Sagittis vitae et leo duis ut
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Pretium lectus quam id leo in
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Fusce ut placerat orci nulla pellentesque dignissim enim sit
     </li>
-    <li>
+    <li
+      draggable="true"
+    >
       Sed cras ornare arcu dui vivamus arcu.
     </li>
   </ol>

diff --git a/renderer/actions/auto-scrollable/auto-scrollable.stories.js b/renderer/actions/auto-scrollable/auto-scrollable.stories.js
@@ -1,6 +1,6 @@
 'use strict'
 
-import { withKnobs, number, boolean } from '@storybook/addon-knobs'
+import { withKnobs, number } from '@storybook/addon-knobs'
 import Scrollable from './auto-scrollable.stories.svelte'
 
 export default {
@@ -11,8 +11,7 @@ export default {
 export const Default = () => ({
   Component: Scrollable,
   props: {
-    enabled: boolean('Enabled', true),
     borderDetection: number('Border detection', 75),
-    maxScroll: number('Max scroll', 50)
+    maxScroll: number('Max scroll', 30)
   }
 })
diff --git a/renderer/actions/auto-scrollable/auto-scrollable.stories.svelte b/renderer/actions/auto-scrollable/auto-scrollable.stories.svelte
@@ -54,10 +54,14 @@
   }
 </style>
 
+<p class="p-4 font-bold">
+  Caution: does not work on Firefox (DragEvent always have 0 clientY)
+</p>
+
 <ol
   use:autoScrollable={$$restProps}
   style="--padding:{$$restProps.borderDetection}px; --margin:{-$$restProps.borderDetection}px;">
   {#each texts as text}
-    <li>{text}</li>
+    <li draggable="true">{text}</li>
   {/each}
 </ol>
diff --git a/renderer/actions/auto-scrollable/auto-scrollable.test.js b/renderer/actions/auto-scrollable/auto-scrollable.test.js
@@ -10,13 +10,21 @@ describe('autoScrollable action', () => {
   let clientHeight
   let scrollHeight
   let scrollTop
+  let clientY
 
   beforeAll(() => {
     Object.defineProperties(window.HTMLOListElement.prototype, {
       clientHeight: { get: () => clientHeight },
       scrollHeight: { get: () => scrollHeight },
       scrollTop: { get: () => scrollTop }
     })
+    // JSDom does not support DragEvent, and falls back to Event instead
+    Object.defineProperty(Event.prototype, 'clientY', {
+      enumerable: true,
+      get() {
+        return clientY
+      }
+    })
   })
 
   beforeEach(() => {
@@ -32,12 +40,12 @@ describe('autoScrollable action', () => {
     const list = items[0].closest('ol')
     list.scrollBy = jest.fn()
 
-    await fireEvent.mouseEnter(list)
-    await fireEvent.mouseMove(items[10], { clientY: clientHeight - 15 })
-    await fireEvent.mouseLeave(list)
+    clientY = clientHeight - 15
+    fireEvent.drag(items[10])
+    fireEvent.dragEnd(list)
     await sleep()
 
-    expect(list.scrollBy).toHaveBeenCalledWith({ top: 40 })
+    expect(list.scrollBy).toHaveBeenCalledWith({ top: 24 })
   })
 
   it('does not scrolls down when already at the bottom', async () => {
@@ -47,9 +55,9 @@ describe('autoScrollable action', () => {
     const list = items[0].closest('ol')
     list.scrollBy = jest.fn()
 
-    await fireEvent.mouseEnter(list)
-    await fireEvent.mouseMove(items[20], { clientY: clientHeight - 15 })
-    await fireEvent.mouseLeave(list)
+    clientY = clientHeight - 15
+    fireEvent.drag(items[20])
+    fireEvent.dragEnd(list)
     await sleep()
 
     expect(list.scrollBy).not.toHaveBeenCalled()
@@ -62,12 +70,12 @@ describe('autoScrollable action', () => {
     const list = items[0].closest('ol')
     list.scrollBy = jest.fn()
 
-    await fireEvent.mouseEnter(list)
-    await fireEvent.mouseMove(items[10], { clientY: 15 })
-    await fireEvent.mouseLeave(list)
+    clientY = 15
+    fireEvent.drag(items[10])
+    fireEvent.dragEnd(list)
     await sleep()
 
-    expect(list.scrollBy).toHaveBeenCalledWith({ top: -40 })
+    expect(list.scrollBy).toHaveBeenCalledWith({ top: -24 })
   })
 
   it('does not scrolls up when already at the top', async () => {
@@ -76,9 +84,9 @@ describe('autoScrollable action', () => {
     const list = items[0].closest('ol')
     list.scrollBy = jest.fn()
 
-    await fireEvent.mouseEnter(list)
-    await fireEvent.mouseMove(items[0], { clientY: 15 })
-    await fireEvent.mouseLeave(list)
+    clientY = 15
+    fireEvent.drag(items[0])
+    fireEvent.dragEnd(list)
     await sleep()
 
     expect(list.scrollBy).not.toHaveBeenCalled()
@@ -91,10 +99,11 @@ describe('autoScrollable action', () => {
     const list = items[0].closest('ol')
     list.scrollBy = jest.fn()
 
-    await fireEvent.mouseEnter(list)
-    await fireEvent.mouseMove(items[0], { clientY: 15 })
-    await fireEvent.mouseMove(items[29], { clientY: clientHeight - 15 })
-    await fireEvent.mouseLeave(list)
+    clientY = 15
+    fireEvent.drag(items[0])
+    clientY = clientHeight - 15
+    fireEvent.drag(items[29])
+    fireEvent.dragEnd(list)
     await sleep()
 
     expect(list.scrollBy).not.toHaveBeenCalled()
@@ -106,25 +115,9 @@ describe('autoScrollable action', () => {
     const list = items[0].closest('ol')
     list.scrollBy = jest.fn()
 
-    await fireEvent.mouseEnter(list)
-    await fireEvent.mouseMove(items[5], { clientY: itemHeight * 5 })
-    await fireEvent.mouseLeave(list)
-    await sleep()
-
-    expect(list.scrollBy).not.toHaveBeenCalled()
-  })
-
-  it('does not scroll when disabled', async () => {
-    scrollTop = 10 * itemHeight
-    render(html`<${Scrollable} enabled=${false} />`)
-    const items = screen.queryAllByRole('listitem')
-    const list = items[0].closest('ol')
-    list.scrollBy = jest.fn()
-
-    await fireEvent.mouseEnter(list)
-    await fireEvent.mouseMove(items[10], { clientY: 15 })
-    await fireEvent.mouseMove(items[20], { clientY: clientHeight - 15 })
-    await fireEvent.mouseLeave(list)
+    clientY = itemHeight * 5
+    fireEvent.drag(items[5])
+    fireEvent.dragEnd(list)
     await sleep()
 
     expect(list.scrollBy).not.toHaveBeenCalled()