Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Table Input Widget: Size persistence #11435

Merged
merged 7 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
- [Changed the way of adding new column in Table Input Widget][11388]. The
"virtual column" is replaced with an explicit (+) button.
- [New dropdown-based component menu][11398].
- [Size of Table Input Widget is preserved and restored after project
re-opening][11435]

[11151]: https://github.com/enso-org/enso/pull/11151
[11271]: https://github.com/enso-org/enso/pull/11271
Expand All @@ -20,6 +22,7 @@
[11383]: https://github.com/enso-org/enso/pull/11383
[11388]: https://github.com/enso-org/enso/pull/11388
[11398]: https://github.com/enso-org/enso/pull/11398
[11435]: https://github.com/enso-org/enso/pull/11435

#### Enso Standard Library

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,23 @@ function handleWidgetUpdates(update: WidgetUpdate) {
selectNode()
const edit = update.edit ?? graph.startEdit()
if (update.portUpdate) {
const { value, origin } = update.portUpdate
const { origin } = update.portUpdate
if (Ast.isAstId(origin)) {
const ast =
value instanceof Ast.Ast ? value
: value == null ? Ast.Wildcard.new(edit)
: undefined
if (ast) {
edit.replaceValue(origin, ast)
} else if (typeof value === 'string') {
edit.tryGet(origin)?.syncToCode(value)
if ('value' in update.portUpdate) {
const value = update.portUpdate.value
const ast =
value instanceof Ast.Ast ? value
: value == null ? Ast.Wildcard.new(edit)
: undefined
if (ast) {
edit.replaceValue(origin, ast)
} else if (typeof value === 'string') {
edit.tryGet(origin)?.syncToCode(value)
}
}
if ('metadata' in update.portUpdate) {
const { metadataKey, metadata } = update.portUpdate
edit.tryGet(origin)?.setWidgetMetadata(metadataKey, metadata)
}
} else {
console.error(`[UPDATE ${origin}] Invalid top-level origin. Expected expression ID.`)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ const innerInput = computed(() => {
function handleArgUpdate(update: WidgetUpdate): boolean {
const app = application.value
if (update.portUpdate && app instanceof ArgumentApplication) {
if (!('value' in update.portUpdate)) {
if (!Ast.isAstId(update.portUpdate.origin))
console.error('Tried to set metadata on arg placeholder. This is not implemented yet!')
return false
}
const { value, origin } = update.portUpdate
const edit = update.edit ?? graph.startEdit()
// Find the updated argument by matching origin port/expression with the appropriate argument.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,29 @@ import type {
} from 'ag-grid-enterprise'
import { computed, markRaw, ref } from 'vue'
import type { ComponentExposed } from 'vue-component-type-helpers'
import { z } from 'zod'

const props = defineProps(widgetProps(widgetDefinition))
const graph = useGraphStore()
const suggestionDb = useSuggestionDbStore()
const grid = ref<ComponentExposed<typeof AgGridTableView<RowData, any>>>()

const configSchema = z.object({ size: z.object({ x: z.number(), y: z.number() }) }).passthrough()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why passthrough? We wouldn't be able to make any use of unrecognized keys, so I think we could keep the default behavior (allowing them to be stripped out during parsing).

type Config = z.infer<typeof configSchema>

const DEFAULT_CFG: Config = { size: { x: 200, y: 150 } }

const config = computed(() => {
const configObj = props.input.value.widgetMetadata('WidgetTableEditor')
if (configObj == null) return DEFAULT_CFG
const parsed = configSchema.safeParse(configObj)
if (parsed.success) return parsed.data
else {
console.warn('Table Editor Widget: could not read config; invalid format: ', parsed.error)
return DEFAULT_CFG
}
})

const { rowData, columnDefs, moveColumn, moveRow, pasteFromClipboard } = useTableNewArgument(
() => props.input,
graph,
Expand Down Expand Up @@ -131,15 +148,22 @@ const headerEditHandler = new HeaderEditing()

// === Resizing ===

const size = ref(new Vec2(200, 150))
const graphNav = injectGraphNavigator()

const size = computed(() => Vec2.FromXY(config.value.size))

const clientBounds = computed({
get() {
return new Rect(Vec2.Zero, size.value.scale(graphNav.scale))
},
set(value) {
size.value = new Vec2(value.width / graphNav.scale, value.height / graphNav.scale)
props.onUpdate({
portUpdate: {
origin: props.input.portId,
metadataKey: 'WidgetTableEditor',
metadata: { size: { x: value.width / graphNav.scale, y: value.height / graphNav.scale } },
},
})
},
})

Expand Down
25 changes: 14 additions & 11 deletions app/gui/src/project-view/providers/widgetRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ import type { WidgetEditHandlerParent } from './widgetRegistry/editHandler'
export type WidgetComponent<T extends WidgetInput> = Component<WidgetProps<T>>

export namespace WidgetInput {
/** TODO: Add docs */
/** Create a basic {@link WidgetInput } from AST node. */
export function FromAst<A extends Ast.Ast | Ast.Token>(ast: A): WidgetInput & { value: A } {
return {
portId: ast.id,
value: ast,
}
}

/** TODO: Add docs */
/** Create a basic {@link WidgetInput } from AST node with enforced port. */
export function FromAstWithPort<A extends Ast.Ast | Ast.Token>(
ast: A,
): WidgetInput & { value: A } {
Expand All @@ -31,7 +31,7 @@ export namespace WidgetInput {
}
}

/** TODO: Add docs */
/** A string representation of widget's value - the code in case of AST value. */
export function valueRepr(input: WidgetInput): string | undefined {
if (typeof input.value === 'string') return input.value
else return input.value?.code()
Expand All @@ -56,24 +56,24 @@ export namespace WidgetInput {
isPlaceholder(input) || input.value instanceof nodeType
}

/** TODO: Add docs */
/** Check if input's value is existing AST node (not placeholder or token). */
export function isAst(input: WidgetInput): input is WidgetInput & { value: Ast.Ast } {
return input.value instanceof Ast.Ast
}

/** Rule out token inputs. */
/** Check if input's value is existing AST node or placeholder. Rule out token inputs. */
export function isAstOrPlaceholder(
input: WidgetInput,
): input is WidgetInput & { value: Ast.Ast | string | undefined } {
return isPlaceholder(input) || isAst(input)
}

/** TODO: Add docs */
/** Check if input's value is an AST token. */
export function isToken(input: WidgetInput): input is WidgetInput & { value: Ast.Token } {
return input.value instanceof Ast.Token
}

/** TODO: Add docs */
/** Check if input's value is an AST which potentially may be a function call. */
export function isFunctionCall(
input: WidgetInput,
): input is WidgetInput & { value: Ast.App | Ast.Ident | Ast.PropertyAccess | Ast.OprApp } {
Expand Down Expand Up @@ -163,15 +163,18 @@ export interface WidgetProps<T> {
* port may not represent any existing AST node) with `edit` containing any additional modifications
* (like inserting necessary imports).
*
* The same way widgets may set their metadata (as this is also technically an AST modification).
* Every widget type should set it's name as `metadataKey`.
*
* The handlers interested in a specific port update should apply it using received edit. The edit
* is committed in {@link NodeWidgetTree}.
*/
export interface WidgetUpdate {
edit?: MutableModule | undefined
portUpdate?: {
value: Ast.Owned | string | undefined
origin: PortId
}
portUpdate?: { origin: PortId } & (
| { value: Ast.Owned | string | undefined }
| { metadataKey: string; metadata: unknown }
)
}

/**
Expand Down
12 changes: 11 additions & 1 deletion app/gui/src/project-view/util/ast/reactive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,23 @@ import { markRaw, shallowReactive } from 'vue'
import { MutableModule } from 'ydoc-shared/ast'
import * as Y from 'yjs'

/** TODO: Add docs */
/**
* Make AST structures inside the module reactive (including the node's and widgets' metadata).
*
* Note that non-Ast structured fields (e.g. ArgumentDefinition) are not themselves reactive --
* an access is tracked when obtaining the object from the Ast, not when accessing the inner
* object's fields.
*/
export function reactiveModule(doc: Y.Doc, onCleanup: (f: () => void) => void): MutableModule {
const module = markRaw(new MutableModule(doc))
const handle = module.observe((update) => {
update.nodesAdded.forEach((astId) => {
const fields = module.get(astId).fields
;(fields as any)._map = shallowReactive((fields as any)._map)
const metadata = fields.get('metadata')
;(metadata as any)._map = shallowReactive((metadata as any)._map)
const widgetsMetadata = metadata.get('widget')
;(widgetsMetadata as any)._map = shallowReactive((widgetsMetadata as any)._map)
})
})
onCleanup(() => module.unobserve(handle))
Expand Down
17 changes: 12 additions & 5 deletions app/ydoc-server/src/edits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const MAX_SIZE_FOR_NORMAL_DIFF = 30000
interface AppliedUpdates {
newCode: string | undefined
newIdMap: IdMap | undefined
newMetadata: fileFormat.IdeMetadata['node'] | undefined
newMetadata: fileFormat.IdeMetadata | undefined
}

/** Return an object containing updated versions of relevant fields, given an update payload. */
Expand All @@ -49,7 +49,7 @@ export function applyDocumentUpdates(
): AppliedUpdates {
const codeChanged = update.nodesUpdated.size || update.nodesAdded.size || update.nodesDeleted.size
let idsChanged = false
let metadataChanged = false
let metadataChanged = update.widgetMetadataUpdated.size > 0
for (const { changes } of update.metadataUpdated) {
for (const [key] of changes) {
if (key === 'externalId') {
Expand All @@ -63,7 +63,7 @@ export function applyDocumentUpdates(

let newIdMap = undefined
let newCode = undefined
let newMetadata = undefined
let newMetadata: fileFormat.IdeMetadata | undefined = undefined

const syncModule = new MutableModule(doc.ydoc)
const root = syncModule.root()
Expand All @@ -76,19 +76,26 @@ export function applyDocumentUpdates(
if (codeChanged || idsChanged || metadataChanged) {
// Update the metadata object.
// Depth-first key order keeps diffs small.
newMetadata = {} satisfies fileFormat.IdeMetadata['node']
newMetadata = { node: {}, widget: {} }
root.visitRecursiveAst(ast => {
let pos = ast.nodeMetadata.get('position')
const vis = ast.nodeMetadata.get('visualization')
const colorOverride = ast.nodeMetadata.get('colorOverride')
if (vis && !pos) pos = { x: 0, y: 0 }
if (pos) {
newMetadata![ast.externalId] = {
newMetadata!.node[ast.externalId] = {
position: { vector: [Math.round(pos.x), Math.round(-pos.y)] },
visualization: vis && translateVisualizationToFile(vis),
colorOverride,
}
}
const widgets = ast.widgetsMetadata()
if (!widgets.entries().next().done) {
if (newMetadata!.widget == null) newMetadata!.widget = {}
newMetadata!.widget[ast.externalId] = Object.fromEntries(
widgets.entries() as IterableIterator<[string, Record<string, unknown>]>,
)
}
})
}

Expand Down
6 changes: 2 additions & 4 deletions app/ydoc-server/src/fileFormat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,12 @@ export const nodeMetadata = z
})
.passthrough()

export type ImportMetadata = z.infer<typeof importMetadata>
export const importMetadata = z.object({}).passthrough()

export type IdeMetadata = z.infer<typeof ideMetadata>
export const ideMetadata = z
.object({
node: z.record(z.string().uuid(), nodeMetadata),
import: z.record(z.string(), importMetadata),
snapshot: z.string().optional(),
widget: z.optional(z.record(z.string().uuid(), z.record(z.string(), z.unknown()))),
})
.passthrough()
.default(() => defaultMetadata().ide)
Expand Down Expand Up @@ -87,6 +84,7 @@ function defaultMetadata() {
ide: {
node: {},
import: {},
widget: {},
},
}
}
Expand Down
32 changes: 23 additions & 9 deletions app/ydoc-server/src/languageServerSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -474,12 +474,14 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {

private static getIdMapToPersist(
idMap: IdMap | undefined,
metadata: fileFormat.IdeMetadata['node'],
metadata: fileFormat.IdeMetadata,
): IdMap | undefined {
if (idMap === undefined) {
return
} else {
const entriesIntersection = idMap.entries().filter(([, id]) => id in metadata)
const entriesIntersection = idMap
.entries()
.filter(([, id]) => id in metadata.node || id in (metadata.widget ?? {}))
return new IdMap(entriesIntersection)
}
}
Expand All @@ -496,7 +498,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
synced: EnsoFileParts,
newCode: string | undefined,
newIdMap: IdMap | undefined,
newMetadata: fileFormat.IdeMetadata['node'] | undefined,
newMetadata: fileFormat.IdeMetadata | undefined,
) {
if (this.syncedContent == null || this.syncedVersion == null) return

Expand All @@ -508,14 +510,13 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
json.stringify({
...this.syncedMeta,
ide: {
...this.syncedMeta.ide,
...newMetadata,
...newSnapshot,
node: newMetadata,
},
})
const idMapToPersist =
(newIdMap || newMetadata) &&
ModulePersistence.getIdMapToPersist(newIdMap, newMetadata ?? this.syncedMeta.ide.node)
ModulePersistence.getIdMapToPersist(newIdMap, newMetadata ?? this.syncedMeta.ide)
const newIdMapToPersistJson = idMapToPersist && serializeIdMap(idMapToPersist)
const code = newCode ?? synced.code
const newContent = combineFileParts({
Expand Down Expand Up @@ -566,7 +567,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
if (!result.ok) return handleError(result.error)
this.syncedContent = newContent
this.syncedVersion = newVersion
if (newMetadata) this.syncedMeta.ide.node = newMetadata
if (newMetadata) this.syncedMeta.ide = newMetadata
if (newCode) this.syncedCode = newCode
if (newIdMapToPersistJson) this.syncedIdMap = newIdMapToPersistJson
if (newMetadataJson) this.syncedMetaJson = newMetadataJson
Expand All @@ -583,6 +584,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
const { code, idMapJson, metadataJson } = contentsReceived
const metadata = fileFormat.tryParseMetadataOrFallback(metadataJson)
const nodeMeta = Object.entries(metadata.ide.node)
const widgetMeta = Object.entries(metadata.ide.widget ?? {})

let parsedSpans
let parsedIdMap
Expand Down Expand Up @@ -646,7 +648,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
(code !== this.syncedCode ||
idMapJson !== this.syncedIdMap ||
metadataJson !== this.syncedMetaJson) &&
nodeMeta.length !== 0
(nodeMeta.length !== 0 || widgetMeta.length !== 0)
) {
const externalIdToAst = new Map<ExternalId, Ast.Ast>()
astRoot.visitRecursiveAst(ast => {
Expand All @@ -671,6 +673,18 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
const newColorOverride = meta.colorOverride
if (oldColorOverride !== newColorOverride) metadata.set('colorOverride', newColorOverride)
}
for (const [id, meta] of widgetMeta) {
if (typeof id !== 'string') continue
const ast = externalIdToAst.get(id as ExternalId)
if (!ast) {
missing.add(id)
continue
}
const widgetsMetadata = syncModule.getVersion(ast).mutableWidgetsMetadata()
for (const [widgetKey, widgetMeta] of Object.entries(meta)) {
widgetsMetadata.set(widgetKey, widgetMeta)
}
}
}

this.syncedCode = code
Expand All @@ -685,7 +699,7 @@ class ModulePersistence extends ObservableV2<{ removed: () => void }> {
contentsReceived,
this.syncedCode ?? undefined,
unsyncedIdMap,
this.syncedMeta?.ide?.node,
this.syncedMeta?.ide,
)
}

Expand Down
Loading
Loading