Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/lib/js/components/columns/column.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export default class Column extends Component {
Sortable.create(childWrap, {
animation: 150,
fallbackClass: 'field-moving',
forceFallback: true,
group: {
name: 'column',
pull: true,
Expand Down
37 changes: 24 additions & 13 deletions src/lib/js/components/controls/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,31 +285,42 @@ export class Controls {
for (let i = groups.length - 1; i >= 0; i--) {
const storeID = `formeo-controls-${groups[i]}`
if (!this.options.sortable) {
window.localStorage.removeItem(storeID)
globalThis.localStorage.removeItem(storeID)
}
Sortable.create(groups[i], {
animation: 150,
forceFallback: true,
fallbackClass: 'control-moving',
fallbackOnBody: true,
forceFallback: true,
fallbackTolerance: 5,
group: {
name: 'controls',
pull: 'clone',
put: false,
revertClone: true,
},
onStart: async ({ item }) => {
const { controlData } = this.get(item.id)
onClone: ({ clone, item }) => {
// Copy the item's id to the clone so we can identify what control it represents
clone.id = item.id

if (this.options.ghostPreview) {
const { controlData } = this.get(item.id)
// Dynamically import Field to avoid circular dependency
const { default: Field } = await import('../fields/field.js')
item.innerHTML = ''
item.appendChild(new Field(controlData).preview)
import('../fields/field.js').then(({ default: Field }) => {
clone.innerHTML = ''
clone.appendChild(new Field(controlData).preview)
})
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dynamic import in onClone lacks error handling. If the import fails, the promise rejection will be unhandled, which could cause issues in some environments or make debugging harder.

Consider adding a .catch() handler:

import('../fields/field.js')
  .then(({ default: Field }) => {
    clone.innerHTML = ''
    clone.appendChild(new Field(controlData).preview)
  })
  .catch(err => console.error('Failed to load Field for ghost preview:', err))
Suggested change
})
}).catch(err => console.error('Failed to load Field for ghost preview:', err))

Copilot uses AI. Check for mistakes.
}
},
onEnd: ({ from, item, clone }) => {
if (from.contains(clone)) {
from.replaceChild(item, clone)
}
onStart: () => {
// Prevent scrollbar flashing during drag by hiding overflow
this.originalDocumentOverflow = document.documentElement.style.overflow
document.documentElement.style.overflow = 'hidden'
},
onEnd: () => {
// Restore overflow after drag completes
document.documentElement.style.overflow = this.originalDocumentOverflow
this.originalDocumentOverflow = null
},
sort: this.options.sortable,
store: {
Expand All @@ -319,7 +330,7 @@ export class Controls {
* @return {Array}
*/
get: () => {
const order = window.localStorage.getItem(storeID)
const order = globalThis.localStorage.getItem(storeID)
return order ? order.split('|') : []
},

Expand All @@ -329,7 +340,7 @@ export class Controls {
*/
set: sortable => {
const order = sortable.toArray()
window.localStorage.setItem(storeID, order.join('|'))
globalThis.localStorage.setItem(storeID, order.join('|'))
},
},
})
Expand Down
1 change: 1 addition & 0 deletions src/lib/js/components/rows/row.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export default class Row extends Component {
Sortable.create(children, {
animation: 150,
fallbackClass: 'column-moving',
forceFallback: true,
group: {
name: 'row',
pull: true,
Expand Down
1 change: 1 addition & 0 deletions src/lib/sass/components/_stage.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
>.children {
@include mixins.display-column;
gap: var.$component-gap;
min-height: 100%;
}

&.removing-all-fields {
Expand Down
171 changes: 171 additions & 0 deletions tests/drag-control-to-stage.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
// @ts-check
import { expect, test } from '@playwright/test'

test.describe('Drag Control to Stage', () => {
test.beforeEach(async ({ page }) => {
await page.goto('/')
// Wait for the editor to load
await expect(page.locator('.formeo-editor')).toBeVisible()
// Wait for controls to be fully initialized
await page.waitForTimeout(500)
})

test('should add a control to stage via click', async ({ page }) => {
// Get the control button
const textInputControl = page.getByRole('button', { name: 'Text Input' })

await expect(textInputControl).toBeVisible()

// Count fields before click
const fieldsBefore = await page.locator('.formeo-field').count()

// Click to add (alternative to drag)
await textInputControl.click()

// Wait for the field to be added
await page.waitForTimeout(300)

// Verify a field was added to the stage
const fieldsAfter = await page.locator('.formeo-field').count()
expect(fieldsAfter).toBeGreaterThan(fieldsBefore)
})

test('should keep control in sidebar after adding to stage', async ({ page }) => {
// Get the control button
const textInputControl = page.getByRole('button', { name: 'Text Input' })

await expect(textInputControl).toBeVisible()

// Add control to stage
await textInputControl.click()

// Wait for animation to complete
await page.waitForTimeout(300)

// Verify the control is still in the sidebar
await expect(textInputControl).toBeVisible()
await expect(textInputControl).toBeEnabled()
})

test('should not cause layout shift when dragging control', async ({ page }) => {
// Get the control groups container
const controlGroups = page.locator('.control-groups')
const textInputControl = page.getByRole('button', { name: 'Text Input' })
const stage = page.locator('.formeo-stage')

await expect(controlGroups).toBeVisible()
await expect(stage).toBeVisible()

// Get initial bounding box of control groups
const initialBounds = await controlGroups.boundingBox()

// Start dragging
await textInputControl.hover()
await page.mouse.down()

// Move towards stage
const stageBounds = await stage.boundingBox()
if (stageBounds && initialBounds) {
await page.mouse.move(stageBounds.x + stageBounds.width / 2, stageBounds.y + stageBounds.height / 2)

// Check that control groups didn't shift significantly during drag
const duringDragBounds = await controlGroups.boundingBox()
if (duringDragBounds) {
// Allow small tolerance for any animation
expect(Math.abs(duringDragBounds.width - initialBounds.width)).toBeLessThan(5)
expect(Math.abs(duringDragBounds.height - initialBounds.height)).toBeLessThan(50)
}
}

// Complete the drop
await page.mouse.up()

// Wait for animation to complete
await page.waitForTimeout(300)

// Verify final bounds are similar to initial
const finalBounds = await controlGroups.boundingBox()
if (finalBounds && initialBounds) {
expect(Math.abs(finalBounds.width - initialBounds.width)).toBeLessThan(5)
}
})

test('should be able to add multiple controls to stage', async ({ page }) => {
// Add multiple different controls via click
const controls = ['Text Input', 'Email', 'Number']

for (const controlName of controls) {
const control = page.getByRole('button', { name: controlName })
await expect(control).toBeVisible()
await control.click()
await page.waitForTimeout(200)
}

// Verify all fields were added
const fields = await page.locator('.formeo-field').count()
expect(fields).toBeGreaterThanOrEqual(controls.length)
})

test('should maintain control functionality after multiple uses', async ({ page }) => {
const textInputControl = page.getByRole('button', { name: 'Text Input' })

// Add first time
await textInputControl.click()
await page.waitForTimeout(300)

const fieldsAfterFirst = await page.locator('.formeo-field').count()

// Add second time - control should still work
await textInputControl.click()
await page.waitForTimeout(300)

const fieldsAfterSecond = await page.locator('.formeo-field').count()

// Should have added another field
expect(fieldsAfterSecond).toBeGreaterThan(fieldsAfterFirst)
})

test('should set overflow hidden during drag to prevent scrollbar flash', async ({ page }) => {
// This test verifies the fix for scrollbar flashing
const textInputControl = page.getByRole('button', { name: 'Text Input' })
const stage = page.locator('.formeo-stage')

await expect(textInputControl).toBeVisible()
await expect(stage).toBeVisible()

// Get stage bounds for drag target
const stageBounds = await stage.boundingBox()

// Start dragging with manual mouse actions
const controlBounds = await textInputControl.boundingBox()
if (controlBounds && stageBounds) {
// Move to control center and start drag
await page.mouse.move(controlBounds.x + controlBounds.width / 2, controlBounds.y + controlBounds.height / 2)
await page.mouse.down()

// Move slightly to trigger the drag
await page.mouse.move(
controlBounds.x + controlBounds.width / 2 + 10,
controlBounds.y + controlBounds.height / 2 + 10
)

// Wait a moment for onStart to fire
await page.waitForTimeout(100)

// Check that overflow is set to hidden during drag
const overflowDuringDrag = await page.evaluate(() => document.documentElement.style.overflow)
expect(overflowDuringDrag).toBe('hidden')

// Complete the drag to stage
await page.mouse.move(stageBounds.x + stageBounds.width / 2, stageBounds.y + stageBounds.height / 2)
await page.mouse.up()

// Wait for drag to complete
await page.waitForTimeout(300)

// Overflow should be restored after drag
const overflowAfterDrag = await page.evaluate(() => document.documentElement.style.overflow)
expect(overflowAfterDrag).not.toBe('hidden')
Comment on lines +167 to +168
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion expect(overflowAfterDrag).not.toBe('hidden') could fail if the original overflow style was an empty string (which is the default). After restoration, it should return to its original value, but this test will pass even if overflow is incorrectly set to 'auto', 'scroll', or any other value.

Consider using a more specific assertion:

expect(overflowAfterDrag).toBe('')

or verify it matches the initial overflow value captured before the test.

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its possible a developer or other code has set a value. we want to prevent breaking any functionality so reverting to whatever it was prior is a safer method.

}
})
})
Loading