Skip to content

Commit

Permalink
fix(clipboard): prevents orphaned connections from being saved to cli…
Browse files Browse the repository at this point in the history
…pboard
  • Loading branch information
jshor committed Mar 24, 2024
1 parent d185ec2 commit a9cda31
Show file tree
Hide file tree
Showing 8 changed files with 132 additions and 41 deletions.
12 changes: 6 additions & 6 deletions src/store/document/actions/__tests__/clipboard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe('clipboard actions', () => {
const item2 = createItem('item2', ItemType.OutputNode, { portIds: ['port2'], groupId: 'group' })
const item3 = createItem('item3', ItemType.InputNode, { portIds: ['port3'] })
const item4 = createItem('item4', ItemType.OutputNode, { portIds: ['port4'] })
const item5 = createItem('item5', ItemType.OutputNode, { portIds: ['port4'] })
const connection1 = createConnection('connection1', 'port3', 'port4')
const group = createGroup('group', ['item1', 'item2'], { isSelected: true })

Expand All @@ -80,7 +81,8 @@ describe('clipboard actions', () => {
item1,
item2,
item3,
item4
item4,
item5
},
ports: {
port1,
Expand All @@ -98,6 +100,7 @@ describe('clipboard actions', () => {
'item1',
'item2',
'item3',
'item4'
]),
selectedConnectionIds: new Set(['connection1']),
selectedGroupIds: new Set(['group'])
Expand Down Expand Up @@ -139,14 +142,11 @@ describe('clipboard actions', () => {
expect(clipboardData.items).toHaveProperty('item1')
expect(clipboardData.items).toHaveProperty('item2')
expect(clipboardData.items).toHaveProperty('item3')
expect(clipboardData.items).toHaveProperty('item4')
})

it('should not copy non-selected items', () => {
expect(clipboardData.items).not.toHaveProperty('item4')
})

it('should not copy orphaned freeports', () => {
expect(clipboardData.items).not.toHaveProperty('chain2Freeport1')
expect(clipboardData.items).not.toHaveProperty('item5')
})

it('should copy all connections that are connected to selected items at both ends', () => {
Expand Down
45 changes: 27 additions & 18 deletions src/store/document/actions/__tests__/connection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ describe('connection actions', () => {
'getRadialSnapOffset'
])

store.updateConnectionExperiment({ x: 0, y: 0 }, { x: 0, y: 0 })
store.updateConnectionExperiment({ x: 0, y: 0 })

expect(boundaries.getPointBoundary).not.toHaveBeenCalled()
expect(boundaries.getRadialSnapOffset).not.toHaveBeenCalled()
Expand All @@ -533,26 +533,21 @@ describe('connection actions', () => {
it('should move the experimental connection to the given target position', () => {
const sourceId = 'source-id'
const position = { x: 20, y: 30 }
const offset = { x: 5, y: 5 }

store.$patch({
connectionExperiment: {
sourceId,
targetPosition: { x: 10, y: 20 }
}
})
store.updateConnectionExperiment(position, offset)
store.updateConnectionExperiment(position)

expect(store.connectionExperiment!.targetPosition).toEqual({
x: position.x - offset.x,
y: position.y - offset.y
})
expect(store.connectionExperiment!.targetPosition).toEqual(position)
})

it('should snap to the nearest available port', () => {
const sourceId = 'source-id'
const position = { x: 30, y: 40 }
const offset = { x: 0, y: 0 }
const snapPoisition = { x: 33, y: 44 }

store.$patch({
Expand All @@ -569,7 +564,7 @@ describe('connection actions', () => {
boundaries.getPointBoundary(snapPoisition)
]
})
store.updateConnectionExperiment(position, offset)
store.updateConnectionExperiment(position)

expect(store.connectionExperiment!.targetPosition).toEqual(snapPoisition)
})
Expand All @@ -587,7 +582,7 @@ describe('connection actions', () => {
}
})

it('should not do anything if there is no connection experiment', () => {
beforeEach(() => {
store.$reset()

stubAll(store, [
Expand All @@ -596,6 +591,9 @@ describe('connection actions', () => {
'connect',
'clearStatelessInfo'
])
})

it('should not do anything if there is no connection experiment', () => {

store.terminateConnectionExperiment()

Expand All @@ -607,7 +605,6 @@ describe('connection actions', () => {

describe('when the connection experiment is in the neighborhood of a connectable port', () => {
beforeEach(() => {
store.$reset()
store.$patch({
connectionExperiment: {
sourceId,
Expand All @@ -620,13 +617,6 @@ describe('connection actions', () => {
connectablePortIds: new Set([targetId])
})

stubAll(store, [
'cacheState',
'commitCachedState',
'connect',
'clearStatelessInfo'
])

store.terminateConnectionExperiment()
})

Expand All @@ -649,6 +639,25 @@ describe('connection actions', () => {
})
})

it('should reverse the source and the target when the source is an input port', () => {
store.$patch({
connectionExperiment: {
sourceId,
targetPosition: { x: 10, y: 20 }
},
ports: {
[sourceId]: target,
[targetId]: source
},
connectablePortIds: new Set([targetId])
})

store.terminateConnectionExperiment()

expect(store.connect).toHaveBeenCalledTimes(1)
expect(store.connect).toHaveBeenCalledWith({ source: targetId, target: sourceId })
})

describe('when the connection experiment is not near a connectable port', () => {
beforeEach(() => {
store.$reset()
Expand Down
66 changes: 65 additions & 1 deletion src/store/document/actions/__tests__/snapping.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { setActivePinia, createPinia } from 'pinia'
import ItemType from '@/types/enums/ItemType'
import { createItem } from './__helpers__'
import { createItem, createPort, stubAll } from './__helpers__'
import { createDocumentStore } from '../..'
import BoundingBox from '@/types/types/BoundingBox'
import boundaries from '../../geometry/boundaries'
import PortType from '@/types/enums/PortType'

setActivePinia(createPinia())

Expand Down Expand Up @@ -58,4 +60,66 @@ describe('snapping actions', () => {
})
})
})

describe('setControlPointSnapBoundaries', () => {
it('should add linear boundaries for non-excluded control points', () => {
const store = createDocumentStore('document')()
const source = 'source'
const target = 'target'
const portPosition1 = { x: 1, y: 2 }
const portPosition2 = { x: 3, y: 4 }
const portPosition3 = { x: 5, y: 6 }
const portPosition4 = { x: 7, y: 8 }

store.$patch({
connections: {
connection1: { source, target }
},
ports: {
[source]: { position: portPosition1 },
[target]: { position: portPosition2 }
}
})
store.setControlPointSnapBoundaries('connection1', [
{ portPosition: portPosition3 },
{ portPosition: portPosition4 }
], 1)

expect(store.snapBoundaries).toEqual(expect.arrayContaining(boundaries.getLinearBoundaries(portPosition1)))
expect(store.snapBoundaries).toEqual(expect.arrayContaining(boundaries.getLinearBoundaries(portPosition2)))
expect(store.snapBoundaries).toEqual(expect.arrayContaining(boundaries.getLinearBoundaries(portPosition3)))
expect(store.snapBoundaries).toEqual(expect.not.arrayContaining(boundaries.getLinearBoundaries(portPosition4)))
})
})

describe('setConnectionExperimentSnapBoundaries', () => {
const store = createDocumentStore('document')()
const port1 = createPort('port1', 'item1', PortType.Input, { position: { x: 1, y: 2 } })
const port2 = createPort('port2', 'item1', PortType.Input, { position: { x: 3, y: 4 } })
const port3 = createPort('port3', 'item1', PortType.Input, { position: { x: 5, y: 6 } })

beforeEach(() => {
stubAll(store, [
'setConnectablePortIds'
])

store.$reset()
store.$patch({
ports: { port1, port2, port3 }
})
store.connectablePortIds = new Set([port2.id, port3.id])
store.setConnectionExperimentSnapBoundaries(port1.id)
})

it('should set the connectable port IDs', () => {
expect(store.setConnectablePortIds).toHaveBeenCalledTimes(1)
expect(store.setConnectablePortIds).toHaveBeenCalledWith({ portId: port1.id, isDragging: true })
})

it('should set the point snap boundaries', () => {
expect(store.snapBoundaries).not.toEqual(expect.arrayContaining([boundaries.getPointBoundary(port1.position)]))
expect(store.snapBoundaries).toEqual(expect.arrayContaining([boundaries.getPointBoundary(port2.position)]))
expect(store.snapBoundaries).toEqual(expect.arrayContaining([boundaries.getPointBoundary(port3.position)]))
})
})
})
2 changes: 1 addition & 1 deletion src/store/document/actions/__tests__/undoRedo.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ describe('undo/redo actions', () => {

const item1 = createItem('item1', ItemType.InputNode, {
portIds: ['port1'],
clock: new ClockPulse('item1', 1000, LogicValue.TRUE)
clock: new ClockPulse('item1', 1000, LogicValue.TRUE, LogicValue.TRUE)
})
const addedItem1 = createItem('addedItem1', ItemType.InputNode)
const addedItem2 = createItem('addedItem2', ItemType.InputNode)
Expand Down
17 changes: 11 additions & 6 deletions src/store/document/actions/clipboard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ export function cut (this: DocumentStoreInstance) {
* Copies selected editor elements to the clipboard.
*/
export function copy (this: DocumentStoreInstance) {
if (!this.hasSelectedItems) {
return window.api.beep()
}

const items: Record<string, Item> = {}
const ports: Record<string, Port> = {}
const connections: Record<string, Connection> = {}
Expand All @@ -31,13 +27,22 @@ export function copy (this: DocumentStoreInstance) {
this.selectedItemIds.forEach(id => {
items[id] = this.items[id]
items[id].portIds.forEach(portId => {
ports[portId] = this.ports[portId]
ports[portId] = {
...this.ports[portId],
connectedPortIds: []
}
})
})

this
.selectedConnectionIds
.forEach(id => connections[id] = this.connections[id])
.forEach(id => {
const connection = this.connections[id]

if (ports[connection.source] && ports[connection.target]) {
connections[id] = connection
}
})

this
.selectedGroupIds
Expand Down
17 changes: 10 additions & 7 deletions src/store/document/actions/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export function destroyConnectionById (this: DocumentStoreInstance, id: string)
this.ports[source].connectedPortIds.splice(sourceIndex, 1)
this.ports[target].connectedPortIds.splice(targetIndex, 1)

// the input signal to the target port is now cut; set the port's value to 'unknown'
this.ports[target].value = LogicValue.UNKNOWN
this.ports[target].wave?.drawPulseChange(LogicValue.UNKNOWN)

Expand Down Expand Up @@ -217,15 +218,11 @@ export function createConnectionExperiment (this: DocumentStoreInstance, sourceI
* Moves the target of the active connection experiment.
*
* @param position - the new position of the target, in editor coordinates
* @param offset - the offset of the target, in editor coordinates
*/
export function updateConnectionExperiment (this: DocumentStoreInstance, position: Point, offset: Point) {
export function updateConnectionExperiment (this: DocumentStoreInstance, position: Point) {
if (!this.connectionExperiment) return

const { x, y } = fromDocumentToEditorCoordinates(this.canvas, this.viewport, {
x: position.x - offset.x,
y: position.y - offset.y
}, this.zoom)
const { x, y } = fromDocumentToEditorCoordinates(this.canvas, this.viewport, position, this.zoom)
const boundingBox = boundaries.getPointBoundary({ x, y })
const { snapping } = usePreferencesStore()
const tolerance = snapping.snapTolerance.value
Expand Down Expand Up @@ -255,7 +252,13 @@ export function terminateConnectionExperiment (this: DocumentStoreInstance) {

if (target) {
this.cacheState()
this.connect({ source: sourceId, target: target.id })

if (this.ports[sourceId].type === PortType.Input) {
this.connect({ source: target.id, target: sourceId })
} else {
this.connect({ source: sourceId, target: target.id })
}

this.commitCachedState()
}

Expand Down
12 changes: 10 additions & 2 deletions src/store/document/actions/snapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,18 @@ export function setSnapBoundaries (this: DocumentStoreInstance, id: string) {
})()
}

/**
* Sets the vertical and horizontal snap boundaries for a connection's control points.
*
* @param excludeIndex - the index of the control point to exclude (e.g., the one actively being dragged)
*/
export function setControlPointSnapBoundaries (
this: DocumentStoreInstance,
id: string,
connectionId: string,
entries: { portPosition: Point }[],
excludeIndex: number
) {
const { source, target } = this.connections[id]
const { source, target } = this.connections[connectionId]

this.snapBoundaries = entries
.reduce((b, { portPosition }, index) => {
Expand All @@ -44,6 +49,9 @@ export function setControlPointSnapBoundaries (
.concat(boundaries.getLinearBoundaries(this.ports[target].position))
}

/**
* Sets the point snap boundaries for all connectable ports.
*/
export function setConnectionExperimentSnapBoundaries (this: DocumentStoreInstance, sourceId: string) {
this.setConnectablePortIds({ portId: sourceId, isDragging: true })
this.snapBoundaries = []
Expand Down
2 changes: 2 additions & 0 deletions src/store/document/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ export interface DocumentState extends SerializableState {
/**
* Information about the connection "experiment" (i.e., when a user drags a port to try to establish a new connection).
* `null` when no experiment is being performed.
*
* The connection experiment is rendered by [PortItem.vue](../../containers/PortItem.vue).
*/
connectionExperiment: {
/** The ID of the port originating the experiment. */
Expand Down

0 comments on commit a9cda31

Please sign in to comment.