Only send updated attributes in item.update to save some bandwidth an…

…d avoid unnecessary conflicts
raimohanska · Sep 23, 2023 · 10ab6f6 · 10ab6f6
1 parent e407349
commit 10ab6f6
Show file tree

Hide file tree

Showing 12 changed files with 41 additions and 25 deletions.
diff --git a/common/src/board-reducer.ts b/common/src/board-reducer.ts
@@ -18,6 +18,7 @@ import {
     isItemEndPoint,
     isTextItem,
     Item,
+    ItemUpdate,
     MoveItem,
     PersistableBoardItemEvent,
     Point,
@@ -163,6 +164,7 @@ export function boardReducer(
                 {
                     action: "item.update",
                     boardId: board.id,
+                    // TODO: Undo of update could only undo the actually updated fields here
                     items: event.items.map((item) => getItem(board)(item.id)),
                 },
             ]
@@ -294,10 +296,11 @@ function applyFontSize(items: Record<string, Item>, factor: number, itemIds: Id[
     }
 }
 
-function updateItems(current: Record<Id, Item>, updateList: Item[]): Record<Id, Item> {
-    const updated = arrayToRecordById(updateList)
-    const result = { ...current, ...updated }
-    updateList.filter(isContainer).forEach((container) => {
+function updateItems(current: Record<Id, Item>, updateList: ItemUpdate[]): Record<Id, Item> {
+    const updatedItems: Item[] = updateList.map((update) => ({ ...current[update.id], ...update } as Item))
+    const updatedItemMap = arrayToRecordById(updatedItems)
+    const resultMap = { ...current, ...updatedItemMap }
+    updatedItems.filter(isContainer).forEach((container) => {
         const previous = current[container.id]
         if (previous && !equalRect(previous, container)) {
             // Container shape changed -> check items
@@ -308,14 +311,14 @@ function updateItems(current: Record<Id, Item>, updateList: Item[]): Record<Id,
                         containedBy(i, container), // Check all items inside the new bounds
                 )
                 .forEach((item) => {
-                    const newContainer = maybeChangeContainerForItem(item, result)
+                    const newContainer = maybeChangeContainerForItem(item, resultMap)
                     if (newContainer?.id !== item.containerId) {
-                        result[item.id] = { ...item, containerId: newContainer ? newContainer.id : undefined }
+                        resultMap[item.id] = { ...item, containerId: newContainer ? newContainer.id : undefined }
                     }
                 })
         }
     })
-    return result
+    return resultMap
 }
 
 function moveItems(board: Board, event: MoveItem) {

diff --git a/common/src/domain.ts b/common/src/domain.ts
@@ -278,7 +278,8 @@ export type AuthJWTLogin = {
 export type AuthLogout = { action: "auth.logout" }
 export type Ping = { action: "ping" }
 export type AddItem = { action: "item.add"; boardId: Id; items: Item[]; connections: Connection[] }
-export type UpdateItem = { action: "item.update"; boardId: Id; items: Item[] }
+export type UpdateItem = { action: "item.update"; boardId: Id; items: ItemUpdate[] }
+export type ItemUpdate<I extends Item = Item> = Partial<I> & { id: Id }
 export type MoveItem = {
     action: "item.move"
     boardId: Id
@@ -500,6 +501,7 @@ export function getItemIds(e: BoardHistoryEntry | PersistableBoardItemEvent): Id
         case "item.move":
             return e.items.map((i) => i.id)
         case "item.update":
+            return e.items.map((i) => i.id)
         case "item.add":
             return e.items.map((i) => i.id)
         case "item.bootstrap":
@@ -677,14 +679,14 @@ export function getVerticalAlign(align: Align): VerticalAlign {
     return "middle"
 }
 
-export function setHorizontalAlign<I extends TextItem>(item: I, a: HorizontalAlign): I {
+export function setHorizontalAlign<I extends TextItem>(item: I, a: HorizontalAlign): ItemUpdate<I> {
     const letter = a === "left" ? "L" : a === "center" ? "C" : "R"
     const align = `${getAlign(item)[0]}${letter}`
-    return { ...item, align }
+    return { id: item.id, align } as ItemUpdate<I>
 }
 
-export function setVerticalAlign<I extends TextItem>(item: I, a: VerticalAlign): I {
+export function setVerticalAlign<I extends TextItem>(item: I, a: VerticalAlign): ItemUpdate<I> {
     const letter = a === "top" ? "T" : a === "middle" ? "M" : "B"
     const align = `${letter}${getAlign(item)[1]}`
-    return { ...item, align }
+    return { id: item.id, align } as ItemUpdate<I>
 }
diff --git a/frontend/src/board/BoardView.tsx b/frontend/src/board/BoardView.tsx
@@ -136,7 +136,7 @@ export const BoardView = ({
     function onURL(assetId: string, url: string) {
         itemsList.get().forEach((i) => {
             if ((i.type === "image" || i.type === "video") && i.assetId === assetId && i.src != url) {
-                dispatch({ action: "item.update", boardId, items: [{ ...i, src: url }] })
+                dispatch({ action: "item.update", boardId, items: [{ id: i.id, src: url }] })
             }
         })
     }

diff --git a/frontend/src/board/HistoryView.tsx b/frontend/src/board/HistoryView.tsx
@@ -238,12 +238,17 @@ export const HistoryView = ({
                         return null // Ignore all the resizes, recolorings...
                     }
                 }
+
+                const updatedItems = event.items.map(
+                    (update) => ({ ...boardBefore.items[update.id], ...update } as Item),
+                )
+
                 return {
                     timestamp,
                     user,
                     itemIds,
                     kind: "changed",
-                    actionText: `changed ${describeItems(event.items)}`,
+                    actionText: `changed ${describeItems(updatedItems)}`,
                 }
             }
             case "board.rename":

diff --git a/frontend/src/board/ItemView.tsx b/frontend/src/board/ItemView.tsx
@@ -153,7 +153,7 @@ export const ItemView = ({
 
     function TextView({ item }: { item: L.Property<TextItem> }) {
         const textAtom = L.atom(L.view(item, "text"), (text) =>
-            dispatch({ action: "item.update", boardId: board.get().id, items: [{ ...item.get(), text }] }),
+            dispatch({ action: "item.update", boardId: board.get().id, items: [{ id, text }] }),
         )
         const showCoords = false
         const focused = L.view(focus, (f) => getSelectedItemIds(f).has(id))

diff --git a/frontend/src/board/SelectionBorder.tsx b/frontend/src/board/SelectionBorder.tsx
@@ -90,7 +90,7 @@ export const SelectionBorder = ({
                             vertical === "top" ? dragStartPosition.height - yDiff : dragStartPosition.height + yDiff,
                         )
                         const updatedItem = {
-                            ...current,
+                            id: current.id,
                             x,
                             y,
                             width,

diff --git a/frontend/src/board/contextmenu/alignments.tsx b/frontend/src/board/contextmenu/alignments.tsx
@@ -76,7 +76,7 @@ function moveFocusedItems(
         .items.sort((item1, item2) => item1[axis] - item2[axis])
         .map((item, index) => {
             const newItem = {
-                ...item,
+                id: item.id,
                 [axis]: getCoordinateToSetToItem(
                     item,
                     min,

diff --git a/frontend/src/board/contextmenu/colors.tsx b/frontend/src/board/contextmenu/colors.tsx
@@ -28,7 +28,7 @@ export function colorsSubMenu({ board, focusedItems, dispatch }: SubmenuProps) {
 
     function setColor(color: Color) {
         const b = board.get()
-        const updated = coloredItems.get().map((item) => ({ ...item, color }))
+        const updated = coloredItems.get().map((item) => ({ id: item.id, color }))
         dispatch({ action: "item.update", boardId: b.id, items: updated })
     }
 }
diff --git a/frontend/src/board/contextmenu/shapes.tsx b/frontend/src/board/contextmenu/shapes.tsx
@@ -58,7 +58,7 @@ export function shapesMenu({ board, focusedItems, dispatch }: SubmenuProps) {
                     newShape === "rect"
                         ? { width: maxDim * 1.2, height: maxDim / 1.2 }
                         : { width: maxDim, height: maxDim }
-                return { ...item, shape: newShape, ...dimensions }
+                return { id: item.id, shape: newShape, ...dimensions }
             }) as ShapedItem[]
             dispatch({ action: "item.update", boardId: b.id, items: updated })
         }

diff --git a/frontend/src/board/contextmenu/textAlignments.tsx b/frontend/src/board/contextmenu/textAlignments.tsx
@@ -5,6 +5,7 @@ import {
     Color,
     HorizontalAlign,
     Item,
+    ItemUpdate,
     TextItem,
     VerticalAlign,
     getAlign,
@@ -37,10 +38,13 @@ export function textAlignmentsMenu({ board, focusedItems, dispatch, submenu }: S
         return getIfSame(f.items, getVerticalAlign, "top")
     })
 
-    function setAlign(modifyAlign: (i: TextItem) => TextItem) {
+    function setAlign(modifyAlign: (i: TextItem) => ItemUpdate<TextItem>) {
         focusedItems.get()
         const b = board.get()
-        const updated = focusedItems.get().items.map((i) => (isTextItem(i) ? modifyAlign(i) : i))
+        const updated = focusedItems
+            .get()
+            .items.filter(isTextItem)
+            .map((i) => modifyAlign(i))
         dispatch({ action: "item.update", boardId: b.id, items: updated })
     }
 

diff --git a/frontend/src/board/item-organizer.ts b/frontend/src/board/item-organizer.ts
@@ -1,4 +1,4 @@
-import { Board, Container, Item } from "../../../common/src/domain"
+import { Board, Container, Item, ItemUpdate } from "../../../common/src/domain"
 import * as G from "../../../common/src/geometry"
 import _ from "lodash"
 
@@ -30,7 +30,8 @@ export function packableItems(cont: Container, board: Board): Item[] {
     return items
 }
 
-export function organizeItems(itemsToPack: Item[], itemsToAvoid: Item[], rect: G.Rect): Item[] {
+// TODO: return only changed attributes in ItemUpdate - currently returns full Items
+export function organizeItems(itemsToPack: Item[], itemsToAvoid: Item[], rect: G.Rect): ItemUpdate[] {
     if (itemsToPack.length === 0) return itemsToPack
     const results: Item[] = []
     let rowY = rect.y

diff --git a/frontend/src/board/item-packer.ts b/frontend/src/board/item-packer.ts
@@ -1,14 +1,14 @@
 // @ts-ignore
 import { BP2D } from "binpackingjs"
 const { Bin, Box, Packer, heuristics } = BP2D
-import { Board, Container, Item } from "../../../common/src/domain"
+import { Board, Container, Item, ItemUpdate } from "../../../common/src/domain"
 import { Rect } from "../../../common/src/geometry"
 import { ITEM_MARGIN } from "./item-organizer"
 
 type PackItemsResult =
     | {
           ok: true
-          packedItems: Item[]
+          packedItems: ItemUpdate[]
       }
     | {
           ok: false
@@ -31,6 +31,7 @@ const PACK_BINARY_SEARCH_DEFAULT: {
     prev: [],
 }
 
+// TODO: return only changed attributes in ItemUpdate - currently returns full Items
 export function packItems(targetRect: Rect, items: Item[], binarySearch = PACK_BINARY_SEARCH_DEFAULT): PackItemsResult {
     const availableWidth = targetRect.width
     const availableHeight = targetRect.height