fix: iframe reload on component selection (#1297)

v3/cypress/e2e/plugin.spec.ts

@@ -142,6 +142,22 @@ context("codap plugins", () => {
     })
   })
 
+  it("doesn't reload the iframe/plugin on selection change", () => {
+    openAPITester()
+
+    const cmd1 = `{
+      "action": "get",
+      "resource": "componentList"
+    }`
+    webView.sendAPITesterCommand(cmd1)
+    webView.confirmAPITesterResponseContains(/"success":\s*true/)
+
+    c.getComponentTitleBar("table").click()
+    c.getComponentTitleBar("codap-web-view").click()
+    // if the prior response is still present, then the iframe wasn't reloaded
+    webView.confirmAPITesterResponseContains(/"success":\s*true/)
+  })
+
   it('will broadcast notifications', () => {
     openAPITester()
     webView.toggleAPITesterFilter()

v3/src/components/container/container.scss

@@ -1,6 +1,8 @@
 @import "../../components/vars";
 
 .codap-container {
+  // set up a stacking context so component z-index values don't interact with others
+  isolation: isolate;
   width: 100%;
   height: calc(100% - $menu-bar-height - $tool-shelf-height);
   background: url("../../assets/bg-grid-grey-v3.png");

v3/src/components/container/free-tile-component.tsx

@@ -23,9 +23,9 @@ export const FreeTileComponent = observer(function FreeTileComponent({ row, tile
   const tileId = tile.id
   const tileType = tile.content.type
   const rowTile = row.tiles.get(tileId)
-  const { x: left, y: top, width, height } = rowTile || {}
+  const { x: left, y: top, width, height, zIndex } = rowTile || {}
   const { active } = useDndContext()
-  const tileStyle: React.CSSProperties = { left, top, width, height }
+  const tileStyle: React.CSSProperties = { left, top, width, height, zIndex }
   const draggableOptions: IUseDraggableTile = { prefix: tileType || "tile", tileId }
   const {setNodeRef, transform} = useDraggableTile(draggableOptions,
     activeDrag => {
@@ -76,7 +76,7 @@ export const FreeTileComponent = observer(function FreeTileComponent({ row, tile
     const onPointerUp = () => {
       document.body.removeEventListener("pointermove", onPointerMove, { capture: true })
       document.body.removeEventListener("pointerup", onPointerUp, { capture: true })
-      mtile.applyModelChange(() => {
+      row.applyModelChange(() => {
         mtile.setSize(resizingWidth, resizingHeight)
         mtile.setPosition(resizingLeft, mtile.y)
       }, {
@@ -88,7 +88,7 @@ export const FreeTileComponent = observer(function FreeTileComponent({ row, tile
 
     document.body.addEventListener("pointermove", onPointerMove, { capture: true })
     document.body.addEventListener("pointerup", onPointerUp, { capture: true })
-  }, [])
+  }, [row])
 
   const handleBottomRightPointerDown = useCallback((e: React.PointerEvent) => {
     rowTile && handleResizePointerDown(e, rowTile, "bottom-right")
@@ -113,8 +113,8 @@ export const FreeTileComponent = observer(function FreeTileComponent({ row, tile
   const startStyleTop = top || 0
   const startStyleLeft = left || 0
   const movingStyle = transform && {top: startStyleTop + transform.y, left: startStyleLeft + transform.x,
-    width, height, transition: "none"}
-  const minimizedStyle = { left, top, width, height: kTitleBarHeight }
+    width, height, zIndex, transition: "none"}
+  const minimizedStyle = { left, top, width, height: kTitleBarHeight, zIndex }
   const style = rowTile?.isMinimized
                   ? minimizedStyle
                   : tileId === resizingTileId

v3/src/models/codap/create-codap-document.test.ts

@@ -30,7 +30,7 @@ describe("createCodapDocument", () => {
     expect(doc.key).toBe("test-1")
     expect(doc.type).toBe("CODAP")
     expect(omitUndefined(getSnapshot(doc.content!))).toEqual({
-      rowMap: { "test-3": { id: "test-3", type: "free", savedOrder: [], tiles: {} } },
+      rowMap: { "test-3": { id: "test-3", type: "free", maxZIndex: 0, tiles: {} } },
       rowOrder: ["test-3"],
       sharedModelMap: {
         "test-2": { sharedModel: { id: "test-2", type: "GlobalValueManager", globals: {} }, tiles: [] }
@@ -67,7 +67,7 @@ describe("createCodapDocument", () => {
 
     // the resulting document content contains the contents of the DataSet
     expect(snapContent).toEqual({
-      rowMap: { "test-3": { id: "test-3", type: "free", savedOrder: [], tiles: {} } },
+      rowMap: { "test-3": { id: "test-3", type: "free", maxZIndex: 0, tiles: {} } },
       rowOrder: ["test-3"],
       sharedModelMap: {
         "test-2": {

v3/src/models/document/free-tile-row.test.ts

@@ -24,7 +24,7 @@ describe("FreeTileRow", () => {
     expect(row.tiles.size).toBe(0)
     expect(row.acceptDefaultInsert).toBe(true)
     expect(row.removeWhenEmpty).toBe(false)
-    expect(row.last).toBe("")
+    expect(row.last).toBeUndefined()
     expect(row.tileCount).toBe(0)
     expect(row.hasTile("foo")).toBe(false)
 
@@ -82,7 +82,7 @@ describe("FreeTileRow", () => {
     row.removeTile("tile-2")
     expect(row.tiles.size).toBe(0)
     expect(row.tiles.get("tile-2")?.tileId).toBeUndefined()
-    expect(row.last).toBe("")
+    expect(row.last).toBeUndefined()
     expect(row.tileIds).toEqual([])
   })
 
@@ -96,19 +96,19 @@ describe("FreeTileRow", () => {
     row.moveTileToTop("tile-2")
     expect(row.tiles.size).toBe(3)
     expect(row.last).toBe("tile-2")
-    expect(row.tileIds).toEqual(["tile-1", "tile-3", "tile-2"])
+    expect(row.tileIds).toEqual(["tile-1", "tile-2", "tile-3"])
 
     // move from beginning to last (top)
     row.moveTileToTop("tile-1")
     expect(row.tiles.size).toBe(3)
     expect(row.last).toBe("tile-1")
-    expect(row.tileIds).toEqual(["tile-3", "tile-2", "tile-1"])
+    expect(row.tileIds).toEqual(["tile-1", "tile-2", "tile-3"])
 
     // move from end to last (nop)
     row.moveTileToTop("tile-1")
     expect(row.tiles.size).toBe(3)
     expect(row.last).toBe("tile-1")
-    expect(row.tileIds).toEqual(["tile-3", "tile-2", "tile-1"])
+    expect(row.tileIds).toEqual(["tile-1", "tile-2", "tile-3"])
   })
 
   it("generates efficient patches", () => {
@@ -127,25 +127,40 @@ describe("FreeTileRow", () => {
     reverses = []
     row.insertTile("tile-3", { x: 100, y: 100, width: 100, height: 100 })
     expect(patches).toEqual([
-      `{"op":"add","path":"/tiles/tile-3","value":{"tileId":"tile-3","x":100,"y":100,"width":100,"height":100}}`
+      `{"op":"replace","path":"/maxZIndex","value":3}`,
+      // eslint-disable-next-line max-len
+      `{"op":"add","path":"/tiles/tile-3","value":{"tileId":"tile-3","x":100,"y":100,"width":100,"height":100,"zIndex":3}}`
     ])
     expect(reverses).toEqual([
+      `{"op":"replace","path":"/maxZIndex","value":2}`,
       `{"op":"remove","path":"/tiles/tile-3"}`
     ])
 
     // move from middle to last (top)
     patches = []
     reverses = []
     row.moveTileToTop("tile-2")
-    expect(patches).toEqual([])
-    expect(reverses).toEqual([])
+    expect(patches).toEqual([
+      `{"op":"replace","path":"/maxZIndex","value":4}`,
+      `{"op":"replace","path":"/tiles/tile-2/zIndex","value":4}`
+    ])
+    expect(reverses).toEqual([
+      `{"op":"replace","path":"/maxZIndex","value":3}`,
+      `{"op":"replace","path":"/tiles/tile-2/zIndex","value":2}`
+    ])
 
     // move from beginning to last (top)
     patches = []
     reverses = []
     row.moveTileToTop("tile-1")
-    expect(patches).toEqual([])
-    expect(reverses).toEqual([])
+    expect(patches).toEqual([
+      `{"op":"replace","path":"/maxZIndex","value":5}`,
+      `{"op":"replace","path":"/tiles/tile-1/zIndex","value":5}`
+    ])
+    expect(reverses).toEqual([
+      `{"op":"replace","path":"/maxZIndex","value":4}`,
+      `{"op":"replace","path":"/tiles/tile-1/zIndex","value":1}`
+    ])
 
     // remove first tile
     patches = []
@@ -155,30 +170,13 @@ describe("FreeTileRow", () => {
       `{"op":"remove","path":"/tiles/tile-3"}`
     ])
     expect(reverses).toEqual([
-      `{"op":"add","path":"/tiles/tile-3","value":{"tileId":"tile-3","x":100,"y":100,"width":100,"height":100}}`
+      // eslint-disable-next-line max-len
+      `{"op":"add","path":"/tiles/tile-3","value":{"tileId":"tile-3","x":100,"y":100,"width":100,"height":100,"zIndex":3}}`
     ])
 
     disposer()
   })
 
-  it("preprocessSnapshot supports legacy and current formats", () => {
-    const legacyRow = FreeTileRow.create({
-      tiles: {
-        "tile-1": { tileId: "tile-1", x: 0, y: 0, width: 100, height: 100 }
-      },
-      order: ["tile-1"]
-    })
-    expect(legacyRow.order).toEqual(["tile-1"])
-
-    const modernRow = FreeTileRow.create({
-      tiles: {
-        "tile-1": { tileId: "tile-1", x: 0, y: 0, width: 100, height: 100 }
-      },
-      savedOrder: ["tile-1"]
-    })
-    expect(modernRow.order).toEqual(["tile-1"])
-  })
-
   it("serializes correctly when prepareSnapshot() is called", () => {
     const row = FreeTileRow.create()
     row.insertTile("tile-1", { x: 0, y: 0, width: 100, height: 100 })
@@ -188,23 +186,23 @@ describe("FreeTileRow", () => {
     expect(getSnapshot(row)).toEqual({
       id: row.id,
       type: "free",
+      maxZIndex: 2,
       tiles: {
-        "tile-1": { tileId: "tile-1", x: 0, y: 0, width: 100, height: 100 },
-        "tile-2": { tileId: "tile-2", x: 50, y: 50, width: 100, height: 100 }
-      },
-      savedOrder: []
+        "tile-1": { tileId: "tile-1", x: 0, y: 0, width: 100, height: 100, zIndex: 1 },
+        "tile-2": { tileId: "tile-2", x: 50, y: 50, width: 100, height: 100, zIndex: 2 }
+      }
     })
 
     // after prepareSnapshot(), savedOrder is correct
     row.prepareSnapshot()
     expect(getSnapshot(row)).toEqual({
       id: row.id,
+      maxZIndex: 2,
       type: "free",
       tiles: {
-        "tile-1": { tileId: "tile-1", x: 0, y: 0, width: 100, height: 100 },
-        "tile-2": { tileId: "tile-2", x: 50, y: 50, width: 100, height: 100 }
-      },
-      savedOrder: ["tile-1", "tile-2"]
+        "tile-1": { tileId: "tile-1", x: 0, y: 0, width: 100, height: 100, zIndex: 1 },
+        "tile-2": { tileId: "tile-2", x: 50, y: 50, width: 100, height: 100, zIndex: 2 }
+      }
     })
   })
 

v3/src/models/document/free-tile-row.ts

@@ -1,8 +1,6 @@
-import { observable } from "mobx"
-import { Instance, SnapshotIn, addDisposer, onPatch, types } from "mobx-state-tree"
+import { Instance, SnapshotIn, types } from "mobx-state-tree"
 import { ITileInRowOptions, ITileRowModel, TileRowModel } from "./tile-row"
 import { withoutUndo } from "../history/without-undo"
-import { applyModelChange } from "../history/apply-model-change"
 
 /*
   Represents the layout of a set of tiles/components with arbitrary rectangular bounds that can
@@ -20,6 +18,7 @@ export const FreeTileLayout = types.model("FreeTileLayout", {
   y: types.number,
   width: types.maybe(types.number),
   height: types.maybe(types.number),
+  zIndex: types.maybe(types.number),
   isHidden: types.maybe(types.boolean),
   isMinimized: types.maybe(types.boolean)
 })
@@ -40,6 +39,9 @@ export const FreeTileLayout = types.model("FreeTileLayout", {
     self.width = width
     self.height = height
   },
+  setZIndex(zIndex: number) {
+    self.zIndex = Math.trunc(zIndex)
+  },
   setHidden(isHidden: boolean) {
     // only store it if it's true
     self.isHidden = isHidden || undefined
@@ -49,7 +51,6 @@ export const FreeTileLayout = types.model("FreeTileLayout", {
     self.isMinimized = isMinimized || undefined
   }
 }))
-.actions(applyModelChange)
 
 export interface IFreeTileLayout extends Instance<typeof FreeTileLayout> {}
 export interface IFreeTileLayoutSnapshot extends SnapshotIn<typeof FreeTileLayout> {}
@@ -64,6 +65,7 @@ export interface IFreeTileInRowOptions extends ITileInRowOptions {
   y: number
   width?: number
   height?: number
+  zIndex?: number
 }
 export const isFreeTileInRowOptions = (options?: ITileInRowOptions): options is IFreeTileInRowOptions =>
               !!options && ("x" in options && options.x != null) && ("y" in options && options.y != null)
@@ -77,15 +79,8 @@ export const FreeTileRow = TileRowModel
   .props({
     type: types.optional(types.literal("free"), "free"),
     tiles: types.map(FreeTileLayout), // tile id => layout
-    savedOrder: types.array(types.string)  // tile ids ordered from back to front
-  })
-  .preProcessSnapshot((snap: any) => {
-    const { order, ...others } = snap
-    return order ? { savedOrder: order, ...others } : snap
+    maxZIndex: 0
   })
-  .volatile(self => ({
-    order: observable.array<string>()
-  }))
   .views(self => ({
     get acceptDefaultInsert() {
       return true
@@ -100,11 +95,19 @@ export const FreeTileRow = TileRowModel
   .views(self => ({
     // id of last (top) node in list
     get last() {
-      return self.order.length > 0 ? self.order[self.order.length - 1] : ""
+      let topTileId: string | undefined
+      let topZIndex = 0
+      self.tiles.forEach(tileLayout => {
+        if ((tileLayout.zIndex ?? 0) > topZIndex) {
+          topTileId = tileLayout.tileId
+          topZIndex = tileLayout.zIndex ?? 0
+        }
+      })
+      return topTileId
     },
-    // returns tile ids in list/traversal order
+    // returns tile ids in traversal order
     get tileIds() {
-      return self.order
+      return Array.from(self.tiles.keys())
     },
     get tileCount() {
       return self.tiles.size
@@ -121,46 +124,23 @@ export const FreeTileRow = TileRowModel
     }
   }))
   .actions(self => ({
-    afterCreate() {
-      // initialize volatile order from savedOrder on creation
-      self.order.replace([...self.savedOrder])
-
-      addDisposer(self, onPatch(self, ({ op, path }) => {
-        // update order whenever tiles are added/removed
-        if (op === "add" || op === "remove") {
-          const match = /^\/tiles\/(.+)$/.exec(path)
-          const tileId = match?.[1]
-          if (tileId) {
-            // newly added tiles should be front-most
-            if (op === "add") {
-              self.order.push(tileId)
-            }
-            // removed tiles should be removed from order
-            else {
-              self.order.remove(tileId)
-            }
-          }
-        }
-      }))
+    nextZIndex() {
+      return ++self.maxZIndex
     },
-    prepareSnapshot() {
-      withoutUndo({ suppressWarning: true })
-      self.savedOrder.replace(self.order)
+    setMaxZIndex(zIndex: number) {
+      self.maxZIndex = zIndex
     },
     insertTile(tileId: string, options?: ITileInRowOptions) {
-      const { x = 50, y = 50, width = undefined, height = undefined } = isFreeTileInRowOptions(options) ? options : {}
-      self.tiles.set(tileId, { tileId, x, y, width, height })
+      const { x = 50, y = 50, width = undefined, height = undefined, zIndex = this.nextZIndex() } =
+        isFreeTileInRowOptions(options) ? options : {}
+      self.tiles.set(tileId, { tileId, x, y, width, height, zIndex })
     },
     removeTile(tileId: string) {
       self.tiles.delete(tileId)
     },
     moveTileToTop(tileId: string) {
       withoutUndo({ suppressWarning: true })
-      const index = self.order.findIndex(id => id === tileId)
-      if ((index >= 0) && (index < self.order.length - 1)) {
-        self.order.splice(index, 1)
-        self.order.push(tileId)
-      }
+      self.getNode(tileId)?.setZIndex(this.nextZIndex())
     },
     setTileDimensions(tileId: string, dimensions: { width?: number, height?: number }) {
       const freeTileLayout = self.getNode(tileId)

v3/src/models/document/tile-row.ts

@@ -1,5 +1,6 @@
 import { Instance, SnapshotIn, SnapshotOut, types } from "mobx-state-tree"
 import { typedId } from "../../utilities/js-utils"
+import { applyModelChange } from "../history/apply-model-change"
 
 /*
   Represents the layout of a set of tiles/components within a section of a document. The term
@@ -87,6 +88,7 @@ export const TileRowModel = types
       // "derived" models should override
     }
   }))
+.actions(applyModelChange)
 
 export interface ITileRowModel extends Instance<typeof TileRowModel> {}
 export interface ITileRowSnapshotIn extends SnapshotIn<typeof TileRowModel> {}