From 794db1a7db0aa3ce657bbde237e8313f02bc8278 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Fri, 21 Oct 2022 16:54:34 +0100 Subject: [PATCH] Make DND feel much smoother by persisting the end position of drops, and more performance by properly memoizing some state values --- .../src/components/preview/DNDHandler.svelte | 7 +- packages/client/src/index.js | 9 ++- packages/client/src/stores/dnd.js | 28 ++++--- packages/client/src/stores/index.js | 1 + packages/client/src/stores/screens.js | 79 ++++++++++++------- packages/client/src/utils/computed.js | 26 ++++++ 6 files changed, 107 insertions(+), 43 deletions(-) diff --git a/packages/client/src/components/preview/DNDHandler.svelte b/packages/client/src/components/preview/DNDHandler.svelte index f556346ad2..e04bd81750 100644 --- a/packages/client/src/components/preview/DNDHandler.svelte +++ b/packages/client/src/components/preview/DNDHandler.svelte @@ -57,7 +57,9 @@ } // Reset state - dndStore.actions.reset() + if (!$dndStore.dropped) { + dndStore.actions.reset() + } } // Callback when initially starting a drag on a draggable component @@ -251,7 +253,7 @@ } // Callback when dropping a drag on top of some component - const onDrop = e => { + const onDrop = () => { if (!source || !drop?.parent || drop?.index == null) { return } @@ -299,6 +301,7 @@ } if (legacyDropTarget && legacyDropMode) { + dndStore.actions.markDropped() builderStore.actions.moveComponent( source.id, legacyDropTarget, diff --git a/packages/client/src/index.js b/packages/client/src/index.js index e9c821c54f..e61df37d95 100644 --- a/packages/client/src/index.js +++ b/packages/client/src/index.js @@ -24,10 +24,6 @@ loadSpectrumIcons() let app const loadBudibase = async () => { - if (get(builderStore).clearGridNextLoad) { - builderStore.actions.setDragging(false) - } - // Update builder store with any builder flags builderStore.set({ ...get(builderStore), @@ -45,6 +41,11 @@ const loadBudibase = async () => { location: window["##BUDIBASE_LOCATION##"], }) + // Reset DND state if we completed a successful drop + if (get(dndStore).dropped) { + dndStore.actions.reset() + } + // Set app ID - this window flag is set by both the preview and the real // server rendered app HTML appStore.actions.setAppId(window["##BUDIBASE_APP_ID##"]) diff --git a/packages/client/src/stores/dnd.js b/packages/client/src/stores/dnd.js index 9f17a5aa90..eee338c167 100644 --- a/packages/client/src/stores/dnd.js +++ b/packages/client/src/stores/dnd.js @@ -1,4 +1,5 @@ import { writable, derived } from "svelte/store" +import { computed } from "../utils/computed.js" const createDndStore = () => { const initialState = { @@ -10,6 +11,9 @@ const createDndStore = () => { // Info about where the component would be dropped drop: null, + + // Whether the current drop has been completed successfully + dropped: false, } const store = writable(initialState) @@ -59,6 +63,13 @@ const createDndStore = () => { store.set(initialState) } + const markDropped = () => { + store.update(state => { + state.dropped = true + return state + }) + } + return { subscribe: store.subscribe, actions: { @@ -67,6 +78,7 @@ const createDndStore = () => { updateTarget, updateDrop, reset, + markDropped, }, } } @@ -77,14 +89,12 @@ export const dndStore = createDndStore() // performance by deriving any state that needs to be externally observed. // By doing this and using primitives, we can avoid invalidating other stores // or components which depend on DND state unless values actually change. -export const dndParent = derived(dndStore, $dndStore => $dndStore.drop?.parent) -export const dndIndex = derived(dndStore, $dndStore => $dndStore.drop?.index) -export const dndBounds = derived( +export const dndParent = computed(dndStore, x => x.drop?.parent) +export const dndIndex = computed(dndStore, x => x.drop?.index) +export const dndDropped = computed(dndStore, x => x.dropped) +export const dndBounds = computed(dndStore, x => x.source?.bounds) +export const dndIsDragging = computed(dndStore, x => !!x.source) +export const dndIsNewComponent = computed( dndStore, - $dndStore => $dndStore.source?.bounds + x => x.source?.newComponentType != null ) -export const dndIsNewComponent = derived( - dndStore, - $dndStore => $dndStore.source?.newComponentType != null -) -export const dndIsDragging = derived(dndStore, $dndStore => !!$dndStore.source) diff --git a/packages/client/src/stores/index.js b/packages/client/src/stores/index.js index c431302d43..7a06e55a67 100644 --- a/packages/client/src/stores/index.js +++ b/packages/client/src/stores/index.js @@ -22,6 +22,7 @@ export { dndBounds, dndIsNewComponent, dndIsDragging, + dndDropped, } from "./dnd" // Context stores are layered and duplicated, so it is not a singleton diff --git a/packages/client/src/stores/screens.js b/packages/client/src/stores/screens.js index fdfd5d5673..d9d91ad51a 100644 --- a/packages/client/src/stores/screens.js +++ b/packages/client/src/stores/screens.js @@ -2,7 +2,13 @@ import { derived } from "svelte/store" import { routeStore } from "./routes" import { builderStore } from "./builder" import { appStore } from "./app" -import { dndIndex, dndParent, dndIsNewComponent, dndBounds } from "./dnd.js" +import { + dndIndex, + dndParent, + dndIsNewComponent, + dndBounds, + dndDropped, +} from "./dnd.js" import { RoleUtils } from "@budibase/frontend-core" import { findComponentById, findComponentParent } from "../utils/components.js" import { Helpers } from "@budibase/bbui" @@ -18,6 +24,7 @@ const createScreenStore = () => { dndIndex, dndIsNewComponent, dndBounds, + dndDropped, ], ([ $appStore, @@ -27,6 +34,7 @@ const createScreenStore = () => { $dndIndex, $dndIsNewComponent, $dndBounds, + $dndDropped, ]) => { let activeLayout, activeScreen let screens @@ -64,39 +72,54 @@ const createScreenStore = () => { // Insert DND placeholder if required if (activeScreen && $dndParent && $dndIndex != null) { + const { selectedComponentId } = $builderStore + + // Extract and save the selected component as we need a reference to it + // later, and we may be removing it + let selectedParent = findComponentParent( + activeScreen.props, + selectedComponentId + ) + const selectedComponent = selectedParent?._children?.find( + x => x._id === selectedComponentId + ) + // Remove selected component from tree if we are moving an existing // component - const { selectedComponentId } = $builderStore - if (!$dndIsNewComponent) { - let selectedParent = findComponentParent( - activeScreen.props, - selectedComponentId + if (!$dndIsNewComponent && selectedParent) { + selectedParent._children = selectedParent._children?.filter( + x => x._id !== selectedComponentId ) - if (selectedParent) { - selectedParent._children = selectedParent._children?.filter( - x => x._id !== selectedComponentId - ) - } } - // Insert placeholder component - const placeholder = { - _component: "@budibase/standard-components/container", - _id: DNDPlaceholderID, - _styles: { - normal: { - width: `${$dndBounds?.width || 666}px`, - height: `${$dndBounds?.height || 666}px`, - opacity: 0, - }, - }, - static: true, - } - let parent = findComponentById(activeScreen.props, $dndParent) - if (!parent._children?.length) { - parent._children = [placeholder] + // Insert placeholder component if dragging, or artificially insert + // the dropped component in the new location if the drop completed + let componentToInsert + if ($dndDropped && !$dndIsNewComponent) { + componentToInsert = selectedComponent } else { - parent._children.splice($dndIndex, 0, placeholder) + componentToInsert = { + _component: "@budibase/standard-components/container", + _id: DNDPlaceholderID, + _styles: { + normal: { + width: `${$dndBounds?.width || 400}px`, + height: `${$dndBounds?.height || 200}px`, + opacity: 0, + }, + }, + static: true, + } + } + if (componentToInsert) { + let parent = findComponentById(activeScreen.props, $dndParent) + if (parent) { + if (!parent._children?.length) { + parent._children = [componentToInsert] + } else { + parent._children.splice($dndIndex, 0, componentToInsert) + } + } } } diff --git a/packages/client/src/utils/computed.js b/packages/client/src/utils/computed.js index e69de29bb2..02bf97710a 100644 --- a/packages/client/src/utils/computed.js +++ b/packages/client/src/utils/computed.js @@ -0,0 +1,26 @@ +import { writable } from "svelte/store" + +const getKey = value => { + if (value == null || typeof value !== "object") { + return value + } else { + return JSON.stringify(value) + } +} + +export const computed = (store, getValue) => { + const initialValue = getValue(store) + const computedStore = writable(initialValue) + let lastKey = getKey(initialValue) + + store.subscribe(state => { + const value = getValue(state) + const key = getKey(value) + if (key !== lastKey) { + lastKey = key + computedStore.set(value) + } + }) + + return computedStore +}