fix(app): polish variable dialog states

- Restore the empty collection call to action\n- Fill the table width and prevent variable type hint clipping\n- Let dialogs and popup menus handle Escape before editor shortcuts
This commit is contained in:
Danila Poyarkov 2026-07-18 05:25:40 +03:00
parent e0d8efb2a4
commit 429a7e5ae2
4 changed files with 43 additions and 5 deletions

View file

@ -48,6 +48,7 @@
### Fixes ### Fixes
- Restore the Variables dialog empty state, full-width table layout, unclipped type menu, and expected Escape handling in dialogs and popup menus.
- Pin the patched `websocket-driver` release used through Trystero/Firebase collaboration to resolve a critical protocol-length advisory. - Pin the patched `websocket-driver` release used through Trystero/Firebase collaboration to resolve a critical protocol-length advisory.
- Preserve unrelated Figma prototype, library, export, and raw metadata when editing modeled `.fig` fields, while still overriding stale field-specific payloads. - Preserve unrelated Figma prototype, library, export, and raw metadata when editing modeled `.fig` fields, while still overriding stale field-specific payloads.
- Make canvas text rendering demand missing font faces and verify CJK/Arabic fallback coverage from CanvasKit shaping results instead of coarse script predictions. - Make canvas text rendering demand missing font faces and verify CJK/Arabic fallback coverage from CanvasKit shaping results instead of coarse script predictions.

View file

@ -52,7 +52,10 @@ function opacityBindings(): ShortcutDefinition[] {
function shouldIgnoreShortcut(event: KeyboardEvent, options: KeyboardShortcutOptions) { function shouldIgnoreShortcut(event: KeyboardEvent, options: KeyboardShortcutOptions) {
return ( return (
(event.target instanceof Element && event.target.closest('[data-picker-content]') !== null) || (event.target instanceof Element &&
event.target.closest(
'[data-picker-content], [role="dialog"], [role="listbox"], [role="menu"]'
) !== null) ||
isEditing(event) || isEditing(event) ||
options.inputFocused.value || options.inputFocused.value ||
!!options.store.state.editingTextId || !!options.store.state.editingTextId ||

View file

@ -46,6 +46,7 @@ import type { VariableType } from '@open-pencil/scene-graph'
const open = defineModel<boolean>('open', { default: false }) const open = defineModel<boolean>('open', { default: false })
const cls = useDialogUI({ content: 'flex h-[75vh] w-[800px] max-w-[90vw] flex-col' }) const cls = useDialogUI({ content: 'flex h-[75vh] w-[800px] max-w-[90vw] flex-col' })
const menuCls = useMenuUI({ content: 'w-40' }) const menuCls = useMenuUI({ content: 'w-40' })
const addVariableMenuCls = useMenuUI({ content: 'w-48' })
const variableTable = tv(variableTableTheme) const variableTable = tv(variableTableTheme)
const tableStyles = variableTable() const tableStyles = variableTable()
@ -128,10 +129,11 @@ function resizeHandleClass(resizing: boolean) {
:class="cls.content" :class="cls.content"
> >
<DialogTitle class="sr-only">{{ dialogs.localVariables }}</DialogTitle> <DialogTitle class="sr-only">{{ dialogs.localVariables }}</DialogTitle>
<div v-if="!ctx.hasCollections" class="flex flex-1 flex-col"> <div v-if="!ctx.hasCollections.value" class="flex flex-1 flex-col">
<div class="flex shrink-0 items-center justify-between border-b border-border px-4 py-3"> <div class="flex shrink-0 items-center justify-between border-b border-border px-4 py-3">
<h2 class="text-sm font-semibold text-surface">{{ dialogs.localVariables }}</h2> <h2 class="text-sm font-semibold text-surface">{{ dialogs.localVariables }}</h2>
<DialogClose <DialogClose
:aria-label="dialogs.close"
class="flex size-6 cursor-pointer items-center justify-center rounded border-none bg-transparent text-muted hover:bg-hover hover:text-surface" class="flex size-6 cursor-pointer items-center justify-center rounded border-none bg-transparent text-muted hover:bg-hover hover:text-surface"
> >
<icon-lucide-x class="size-4" /> <icon-lucide-x class="size-4" />
@ -235,6 +237,7 @@ function resizeHandleClass(resizing: boolean) {
</button> </button>
</Tip> </Tip>
<DialogClose <DialogClose
:aria-label="dialogs.close"
class="flex size-6 cursor-pointer items-center justify-center rounded border-none bg-transparent text-muted hover:bg-hover hover:text-surface" class="flex size-6 cursor-pointer items-center justify-center rounded border-none bg-transparent text-muted hover:bg-hover hover:text-surface"
> >
<icon-lucide-x class="size-4" /> <icon-lucide-x class="size-4" />
@ -250,7 +253,7 @@ function resizeHandleClass(resizing: boolean) {
> >
<div class="flex-1 overflow-auto"> <div class="flex-1 overflow-auto">
<table <table
class="w-full border-collapse" class="w-full min-w-full border-collapse"
:style="{ width: `${ctx.table.getCenterTotalSize()}px` }" :style="{ width: `${ctx.table.getCenterTotalSize()}px` }"
> >
<thead class="sticky top-0 z-10 bg-panel"> <thead class="sticky top-0 z-10 bg-panel">
@ -395,7 +398,7 @@ function resizeHandleClass(resizing: boolean) {
side="top" side="top"
:side-offset="8" :side-offset="8"
align="end" align="end"
:class="menuCls.content" :class="addVariableMenuCls.content"
> >
<DropdownMenuItem <DropdownMenuItem
v-for="item in variableTypes" v-for="item in variableTypes"

View file

@ -1,4 +1,5 @@
import { expect, test, useEditorSetup } from '#tests/e2e/fixtures' import { expect, test, useEditorSetup } from '#tests/e2e/fixtures'
import { expectDefined } from '#tests/helpers/assert'
import { variablesAddTestId } from '#tests/helpers/test-ids' import { variablesAddTestId } from '#tests/helpers/test-ids'
const editor = useEditorSetup() const editor = useEditorSetup()
@ -25,12 +26,31 @@ function openVariables() {
.getByRole('button', { name: 'Open variables' }) .getByRole('button', { name: 'Open variables' })
} }
test('empty variables dialog offers to create a collection', async () => {
await openVariables().click()
const dialog = editor.page.getByTestId('variables-dialog')
await expect(dialog).toBeVisible()
await expect(dialog.getByText('No variable collections')).toBeVisible()
await expect(dialog.getByRole('button', { name: 'Create collection' })).toBeVisible()
await editor.page.keyboard.press('Escape')
await expect(dialog).toBeHidden()
})
test('variables dialog opens', async () => { test('variables dialog opens', async () => {
await createColorVariable('primary-color') await createColorVariable('primary-color')
await openVariables().click() await openVariables().click()
await expect(editor.page.getByTestId('variables-dialog')).toBeVisible() const dialog = editor.page.getByTestId('variables-dialog')
await expect(dialog).toBeVisible()
await expect(editor.page.locator('[data-default="true"]')).toHaveCount(1) await expect(editor.page.locator('[data-default="true"]')).toHaveCount(1)
const table = dialog.locator('table')
const tableBox = expectDefined(await table.boundingBox(), 'variables table bounds')
const scrollerBox = expectDefined(
await table.locator('..').boundingBox(),
'table scroller bounds'
)
expect(tableBox.width).toBeGreaterThanOrEqual(scrollerBox.width)
editor.canvas.assertNoErrors() editor.canvas.assertNoErrors()
}) })
@ -55,6 +75,17 @@ test('add variable menu creates non-color variable types', async () => {
await editor.page.getByTestId('variables-search-input').fill('') await editor.page.getByTestId('variables-search-input').fill('')
await editor.canvas.waitForRender() await editor.canvas.waitForRender()
await editor.page.getByTestId('variables-add-variable').click()
const numberOption = editor.page.getByTestId(variablesAddTestId('FLOAT'))
const numberHint = numberOption.getByText('Sizes, spacing, opacity')
await expect(numberHint).toBeVisible()
expect(await numberHint.evaluate((element) => element.scrollWidth <= element.clientWidth)).toBe(
true
)
await editor.page.keyboard.press('Escape')
await expect(numberOption).toBeHidden()
await expect(editor.page.getByTestId('variables-dialog')).toBeVisible()
await editor.page.getByTestId('variables-add-variable').click() await editor.page.getByTestId('variables-add-variable').click()
await editor.page.getByTestId(variablesAddTestId('FLOAT')).click() await editor.page.getByTestId(variablesAddTestId('FLOAT')).click()
await expect( await expect(