Skip to content

Commit

Permalink
Implement the limit of 256 cells in the Table Editor (#11448)
Browse files Browse the repository at this point in the history
  • Loading branch information
farmaazon authored Nov 4, 2024
1 parent 4cb943b commit 2bbd909
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
re-opening][11435]
- [Added application version to the title bar.][11446]
- [Added "open grouped components" action to the context menu.][11447]
- [Table Input Widget has now a limit of 256 cells.][11448]
- [Added an error message screen displayed when viewing a deleted
component.][11452]

Expand All @@ -31,6 +32,7 @@
[11435]: https://github.com/enso-org/enso/pull/11435
[11446]: https://github.com/enso-org/enso/pull/11446
[11447]: https://github.com/enso-org/enso/pull/11447
[11448]: https://github.com/enso-org/enso/pull/11448
[11452]: https://github.com/enso-org/enso/pull/11452

#### Enso Standard Library
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { WidgetInputIsSpecificMethodCall } from '@/components/GraphEditor/widgets/WidgetFunction.vue'
import TableHeader from '@/components/GraphEditor/widgets/WidgetTableEditor/TableHeader.vue'
import {
CELLS_LIMIT,
tableNewCallMayBeHandled,
useTableNewArgument,
type RowData,
Expand All @@ -16,6 +17,7 @@ import { useGraphStore } from '@/stores/graph'
import { useSuggestionDbStore } from '@/stores/suggestionDatabase'
import { Rect } from '@/util/data/rect'
import { Vec2 } from '@/util/data/vec2'
import { useToast } from '@/util/toast'
import '@ag-grid-community/styles/ag-grid.css'
import '@ag-grid-community/styles/ag-theme-alpine.css'
import type {
Expand All @@ -35,6 +37,7 @@ const props = defineProps(widgetProps(widgetDefinition))
const graph = useGraphStore()
const suggestionDb = useSuggestionDbStore()
const grid = ref<ComponentExposed<typeof AgGridTableView<RowData, any>>>()
const pasteWarning = useToast.warning()
const configSchema = z.object({ size: z.object({ x: z.number(), y: z.number() }) })
type Config = z.infer<typeof configSchema>
Expand Down Expand Up @@ -194,10 +197,13 @@ function processDataFromClipboard({ data, api }: ProcessDataFromClipboardParams<
const focusedCell = api.getFocusedCell()
if (focusedCell === null) console.warn('Pasting while no cell is focused!')
else {
pasteFromClipboard(data, {
const pasted = pasteFromClipboard(data, {
rowIndex: focusedCell.rowIndex,
colId: focusedCell.column.getColId(),
})
if (pasted.rows < data.length || pasted.columns < (data[0]?.length ?? 0)) {
pasteWarning.show(`Truncated pasted data to keep table within ${CELLS_LIMIT} limit`)
}
}
return []
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,8 @@ export interface HeaderEditHandlers {
* (not in defaultColumnDef).
*/
export type ColumnSpecificHeaderParams =
| {
type: 'astColumn'
editHandlers: HeaderEditHandlers
}
| { type: 'newColumn'; newColumnRequested: () => void }
| { type: 'astColumn'; editHandlers: HeaderEditHandlers }
| { type: 'newColumn'; enabled?: boolean; newColumnRequested: () => void }
| { type: 'rowIndexColumn' }
/**
Expand Down Expand Up @@ -105,6 +102,7 @@ function onMouseRightClick(event: MouseEvent) {
class="addColumnButton"
name="add"
title="Add new column"
:disabled="!(params.enabled ?? true)"
@click.stop="params.newColumnRequested()"
/>
<div
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
CELLS_LIMIT,
DEFAULT_COLUMN_PREFIX,
NEW_COLUMN_ID,
ROW_INDEX_HEADER,
Expand All @@ -7,7 +8,7 @@ import {
useTableNewArgument,
} from '@/components/GraphEditor/widgets/WidgetTableEditor/tableNewArgument'
import { MenuItem } from '@/components/shared/AgGridTableView.vue'
import { WidgetInput } from '@/providers/widgetRegistry'
import { WidgetInput, WidgetUpdate } from '@/providers/widgetRegistry'
import { SuggestionDb } from '@/stores/suggestionDatabase'
import { makeType } from '@/stores/suggestionDatabase/entry'
import { assert } from '@/util/assert'
Expand All @@ -21,9 +22,17 @@ function suggestionDbWithNothing() {
return db
}

function generateTableOfOnes(rows: number, cols: number) {
const code = `Table.new [${[...Array(cols).keys()].map((i) => `['Column #${i}', [${Array(rows).fill('1').join(',')}]]`).join(',')}]`
return Ast.parse(code)
}

const expectedRowIndexColumnDef = { headerName: ROW_INDEX_HEADER }
const expectedNewColumnDef = { cellStyle: { display: 'none' } }

const CELLS_LIMIT_SQRT = Math.sqrt(CELLS_LIMIT)
assert(CELLS_LIMIT_SQRT === Math.floor(CELLS_LIMIT_SQRT))

test.each([
{
code: 'Table.new [["a", [1, 2, 3]], ["b", [4, 5, "six"]], ["empty", [Nothing, Standard.Base.Nothing, Nothing]]]',
Expand Down Expand Up @@ -112,6 +121,54 @@ test.each([
expect(addMissingImports).not.toHaveBeenCalled()
})

test.each([
{
rows: Math.floor(CELLS_LIMIT / 2) + 1,
cols: 1,
expectNewRowEnabled: true,
expectNewColEnabled: false,
},
{
rows: 1,
cols: Math.floor(CELLS_LIMIT / 2) + 1,
expectNewRowEnabled: false,
expectNewColEnabled: true,
},
{
rows: 1,
cols: CELLS_LIMIT,
expectNewRowEnabled: false,
expectNewColEnabled: false,
},
{
rows: CELLS_LIMIT,
cols: 1,
expectNewRowEnabled: false,
expectNewColEnabled: false,
},
{
rows: CELLS_LIMIT_SQRT,
cols: CELLS_LIMIT_SQRT,
expectNewRowEnabled: false,
expectNewColEnabled: false,
},
])(
'Allowed actions in table near limit (rows: $rows, cols: $cols)',
({ rows, cols, expectNewRowEnabled, expectNewColEnabled }) => {
const input = WidgetInput.FromAst(generateTableOfOnes(rows, cols))
const tableNewArgs = useTableNewArgument(
input,
{ startEdit: vi.fn(), addMissingImports: vi.fn() },
suggestionDbWithNothing(),
vi.fn(),
)
expect(tableNewArgs.rowData.value.length).toBe(rows + (expectNewRowEnabled ? 1 : 0))
const lastColDef = tableNewArgs.columnDefs.value[tableNewArgs.columnDefs.value.length - 1]
assert(lastColDef?.headerComponentParams?.type === 'newColumn')
expect(lastColDef.headerComponentParams.enabled ?? true).toBe(expectNewColEnabled)
},
)

test.each([
'Table.new 14',
'Table.new array1',
Expand Down Expand Up @@ -517,3 +574,35 @@ test.each([
else expect(addMissingImports).not.toHaveBeenCalled()
},
)

test('Pasted data which would exceed cells limit is truncated', () => {
const initialRows = CELLS_LIMIT_SQRT - 2
const initialCols = CELLS_LIMIT_SQRT - 1
const ast = generateTableOfOnes(initialRows, initialCols)
const input = WidgetInput.FromAst(ast)
const startEdit = vi.fn(() => ast.module.edit())
const onUpdate = vi.fn((update) => {
const inputAst = update.edit!.getVersion(ast)
// We expect the table to be fully extended, so the number of cells (numbers or Nothings) should be equal to the limit.
let cellCount = 0
inputAst.visitRecursive((ast: Ast.Ast | Ast.Token) => {
if (ast instanceof Ast.Token) return
if (ast instanceof Ast.NumericLiteral || ast.code() === 'Nothing') cellCount++
})
expect(cellCount).toBe(CELLS_LIMIT)
})
const addMissingImports = vi.fn()
const tableNewArgs = useTableNewArgument(
input,
{ startEdit, addMissingImports },
suggestionDbWithNothing(),
onUpdate,
)
const focusedCol = tableNewArgs.columnDefs.value[initialCols - 2]
assert(focusedCol?.colId != null)
tableNewArgs.pasteFromClipboard(Array(4).fill(Array(4).fill('2')), {
rowIndex: initialRows - 2,
colId: focusedCol.colId,
})
expect(onUpdate).toHaveBeenCalledOnce()
})
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ export const ROW_INDEX_HEADER = '#'
export const DEFAULT_COLUMN_PREFIX = 'Column #'
const NOTHING_PATH = 'Standard.Base.Nothing.Nothing' as QualifiedName
const NOTHING_NAME = qnLastSegment(NOTHING_PATH)
/**
* The cells limit of the table; any modification which would exceed this limt should be
* disallowed in UI
*/
export const CELLS_LIMIT = 256

export type RowData = {
index: number
Expand Down Expand Up @@ -180,10 +185,28 @@ export function useTableNewArgument(
}
}

function mayAddNewRow(
rowCount_: number = rowCount.value,
colCount: number = columns.value.length,
): boolean {
return (rowCount_ + 1) * colCount <= CELLS_LIMIT
}

function mayAddNewColumn(
rowCount_: number = rowCount.value,
colCount: number = columns.value.length,
): boolean {
return rowCount_ * (colCount + 1) <= CELLS_LIMIT
}

function addRow(
edit: Ast.MutableModule,
valueGetter: (column: Ast.AstId, index: number) => unknown = () => null,
) {
if (!mayAddNewRow()) {
console.error(`Cannot add new row: the ${CELLS_LIMIT} limit of cells would be exceeded.`)
return
}
for (const [index, column] of columns.value.entries()) {
const editedCol = edit.getVersion(column.data)
editedCol.push(convertWithImport(valueGetter(column.data.id, index), edit))
Expand All @@ -204,6 +227,10 @@ export function useTableNewArgument(
size: number = rowCount.value,
columns?: Ast.Vector,
) {
if (!mayAddNewColumn()) {
console.error(`Cannot add new column: the ${CELLS_LIMIT} limit of cells would be exceeded.`)
return
}
function* cellsGenerator() {
for (let i = 0; i < size; ++i) {
yield convertWithImport(valueGetter(i), edit)
Expand Down Expand Up @@ -273,6 +300,7 @@ export function useTableNewArgument(
maxWidth: 40,
headerComponentParams: {
type: 'newColumn',
enabled: mayAddNewColumn(),
newColumnRequested: () => {
const edit = graph.startEdit()
fixColumns(edit)
Expand Down Expand Up @@ -382,7 +410,9 @@ export function useTableNewArgument(
}
}
}
rows.push({ index: rows.length, cells: {} })
if (mayAddNewRow()) {
rows.push({ index: rows.length, cells: {} })
}
return rows
})

Expand Down Expand Up @@ -434,7 +464,7 @@ export function useTableNewArgument(
}

function pasteFromClipboard(data: string[][], focusedCell: { rowIndex: number; colId: string }) {
if (data.length === 0) return
if (data.length === 0) return { rows: 0, columns: 0 }
const edit = graph.startEdit()
const focusedColIndex =
findIndexOpt(columns.value, ({ id }) => id === focusedCell.colId) ?? columns.value.length
Expand All @@ -446,6 +476,9 @@ export function useTableNewArgument(
}
const pastedRowsEnd = focusedCell.rowIndex + data.length
const pastedColsEnd = focusedColIndex + data[0]!.length
// First we assume we'll paste all data. If not, these vars will be updated.
let actuallyPastedRowsEnd = pastedRowsEnd
let actuallyPastedColsEnd = pastedColsEnd

// Set data in existing cells.
for (
Expand All @@ -467,11 +500,20 @@ export function useTableNewArgument(
// Extend the table if necessary.
const newRowCount = Math.max(pastedRowsEnd, rowCount.value)
for (let i = rowCount.value; i < newRowCount; ++i) {
if (!mayAddNewRow(i)) {
actuallyPastedRowsEnd = i
break
}

addRow(edit, (_colId, index) => newValueGetter(i, index))
}
const newColCount = Math.max(pastedColsEnd, columns.value.length)
let modifiedColumnsAst: Ast.Vector | undefined
for (let i = columns.value.length; i < newColCount; ++i) {
if (!mayAddNewColumn(newRowCount, i)) {
actuallyPastedColsEnd = i
break
}
modifiedColumnsAst = addColumn(
edit,
`${DEFAULT_COLUMN_PREFIX}${i + 1}`,
Expand All @@ -481,7 +523,10 @@ export function useTableNewArgument(
)
}
onUpdate({ edit })
return
return {
rows: actuallyPastedRowsEnd - focusedCell.rowIndex,
columns: actuallyPastedColsEnd - focusedColIndex,
}
}

return {
Expand Down Expand Up @@ -513,6 +558,8 @@ export function useTableNewArgument(
* If the pasted data are to be placed outside current table, the table is extended.
* @param data the clipboard data, as retrieved in `processDataFromClipboard`.
* @param focusedCell the currently focused cell: will become the left-top cell of pasted data.
* @returns number of actually pasted rows and columns; may be smaller than `data` size in case
* it would exceed {@link CELLS_LIMIT}.
*/
pasteFromClipboard,
}
Expand Down

0 comments on commit 2bbd909

Please sign in to comment.