diff --git a/packages/backend-core/src/db/constants.ts b/packages/backend-core/src/db/constants.ts index bfa7595d62..69c98fe569 100644 --- a/packages/backend-core/src/db/constants.ts +++ b/packages/backend-core/src/db/constants.ts @@ -1,14 +1,5 @@ -export const CONSTANT_INTERNAL_ROW_COLS = [ - "_id", - "_rev", - "type", - "createdAt", - "updatedAt", - "tableId", -] as const - -export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const - -export function isInternalColumnName(name: string): boolean { - return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name) -} +export { + CONSTANT_INTERNAL_ROW_COLS, + CONSTANT_EXTERNAL_ROW_COLS, + isInternalColumnName, +} from "@budibase/shared-core" diff --git a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte index 17ecd8f844..d79eedd194 100644 --- a/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte +++ b/packages/builder/src/components/backend/DataTable/modals/CreateEditColumn.svelte @@ -17,6 +17,8 @@ SWITCHABLE_TYPES, ValidColumnNameRegex, helpers, + CONSTANT_INTERNAL_ROW_COLS, + CONSTANT_EXTERNAL_ROW_COLS, } from "@budibase/shared-core" import { createEventDispatcher, getContext, onMount } from "svelte" import { cloneDeep } from "lodash/fp" @@ -52,7 +54,6 @@ const DATE_TYPE = FieldType.DATETIME const dispatch = createEventDispatcher() - const PROHIBITED_COLUMN_NAMES = ["type", "_id", "_rev", "tableId"] const { dispatch: gridDispatch, rows } = getContext("grid") export let field @@ -487,20 +488,27 @@ }) } const newError = {} + const prohibited = externalTable + ? CONSTANT_EXTERNAL_ROW_COLS + : CONSTANT_INTERNAL_ROW_COLS if (!externalTable && fieldInfo.name?.startsWith("_")) { newError.name = `Column name cannot start with an underscore.` } else if (fieldInfo.name && !fieldInfo.name.match(ValidColumnNameRegex)) { newError.name = `Illegal character; must be alpha-numeric.` - } else if (PROHIBITED_COLUMN_NAMES.some(name => fieldInfo.name === name)) { - newError.name = `${PROHIBITED_COLUMN_NAMES.join( + } else if ( + prohibited.some( + name => fieldInfo?.name?.toLowerCase() === name.toLowerCase() + ) + ) { + newError.name = `${prohibited.join( ", " - )} are not allowed as column names` + )} are not allowed as column names - case insensitive.` } else if (inUse($tables.selected, fieldInfo.name, originalName)) { newError.name = `Column name already in use.` } if (fieldInfo.type === FieldType.AUTO && !fieldInfo.subtype) { - newError.subtype = `Auto Column requires a type` + newError.subtype = `Auto Column requires a type.` } if (fieldInfo.fieldName && fieldInfo.tableId) { diff --git a/packages/frontend-core/src/components/grid/layout/ButtonColumn.svelte b/packages/frontend-core/src/components/grid/layout/ButtonColumn.svelte index 9718ddf65f..2266c20c4e 100644 --- a/packages/frontend-core/src/components/grid/layout/ButtonColumn.svelte +++ b/packages/frontend-core/src/components/grid/layout/ButtonColumn.svelte @@ -3,6 +3,7 @@ import { Button } from "@budibase/bbui" import GridCell from "../cells/GridCell.svelte" import GridScrollWrapper from "./GridScrollWrapper.svelte" + import { BlankRowID } from "../lib/constants" const { renderedRows, @@ -17,6 +18,7 @@ isDragging, buttonColumnWidth, showVScrollbar, + dispatch, } = getContext("grid") let container @@ -91,6 +93,17 @@ {/each} +
($hoveredRowId = BlankRowID)} + on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} + > + dispatch("add-row-inline")} + /> +
@@ -131,8 +144,11 @@ align-items: center; gap: 4px; } + .blank :global(.cell:hover) { + cursor: pointer; + } - /* Add left cell border */ + /* Add left cell border to all cells */ .button-column :global(.cell) { border-left: var(--cell-border); } diff --git a/packages/frontend-core/src/components/grid/layout/Grid.svelte b/packages/frontend-core/src/components/grid/layout/Grid.svelte index 1ec785b166..878a9805c0 100644 --- a/packages/frontend-core/src/components/grid/layout/Grid.svelte +++ b/packages/frontend-core/src/components/grid/layout/Grid.svelte @@ -28,7 +28,7 @@ MaxCellRenderOverflow, GutterWidth, DefaultRowHeight, - Padding, + VPadding, SmallRowHeight, ControlsHeight, ScrollBarSize, @@ -119,7 +119,7 @@ // Derive min height and make available in context const minHeight = derived(rowHeight, $height => { const heightForControls = showControls ? ControlsHeight : 0 - return Padding + SmallRowHeight + $height + heightForControls + return VPadding + SmallRowHeight + $height + heightForControls }) context = { ...context, minHeight } @@ -356,8 +356,13 @@ transition: none; } - /* Overrides */ - .grid.quiet :global(.grid-data-content .row > .cell:not(:last-child)) { + /* Overrides for quiet */ + .grid.quiet :global(.grid-data-content .row > .cell:not(:last-child)), + .grid.quiet :global(.sticky-column .row > .cell), + .grid.quiet :global(.new-row .row > .cell:not(:last-child)) { border-right: none; } + .grid.quiet :global(.sticky-column:before) { + display: none; + } diff --git a/packages/frontend-core/src/components/grid/layout/GridBody.svelte b/packages/frontend-core/src/components/grid/layout/GridBody.svelte index a62de5e2d8..e56db8d088 100644 --- a/packages/frontend-core/src/components/grid/layout/GridBody.svelte +++ b/packages/frontend-core/src/components/grid/layout/GridBody.svelte @@ -2,6 +2,7 @@ import { getContext, onMount } from "svelte" import GridScrollWrapper from "./GridScrollWrapper.svelte" import GridRow from "./GridRow.svelte" + import GridCell from "../cells/GridCell.svelte" import { BlankRowID } from "../lib/constants" import ButtonColumn from "./ButtonColumn.svelte" @@ -46,7 +47,6 @@ -
{#each $renderedRows as row, idx} @@ -54,13 +54,16 @@ {/each} {#if $config.canAddRows}
($hoveredRowId = BlankRowID)} on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} - on:click={() => dispatch("add-row-inline")} - /> + > + dispatch("add-row-inline")} + /> +
{/if}
{#if $props.buttons?.length} @@ -76,15 +79,13 @@ overflow: hidden; flex: 1 1 auto; } - .blank { - height: var(--row-height); - background: var(--cell-background); - border-bottom: var(--cell-border); - border-right: var(--cell-border); - position: absolute; + .row { + display: flex; + flex-direction: row; + justify-content: flex-start; + align-items: stretch; } - .blank.highlighted { - background: var(--cell-background-hover); + .blank :global(.cell:hover) { cursor: pointer; } diff --git a/packages/frontend-core/src/components/grid/layout/NewRow.svelte b/packages/frontend-core/src/components/grid/layout/NewRow.svelte index 91555904d2..5ed15cad73 100644 --- a/packages/frontend-core/src/components/grid/layout/NewRow.svelte +++ b/packages/frontend-core/src/components/grid/layout/NewRow.svelte @@ -32,6 +32,7 @@ inlineFilters, columnRenderMap, visibleColumns, + scrollTop, } = getContext("grid") let visible = false @@ -44,6 +45,21 @@ $: $datasource, (visible = false) $: selectedRowCount = Object.values($selectedRows).length $: hasNoRows = !$rows.length + $: renderedRowCount = $renderedRows.length + $: offset = getOffset($hasNextPage, renderedRowCount, $rowHeight, $scrollTop) + + const getOffset = (hasNextPage, rowCount, rowHeight, scrollTop) => { + // If we have a next page of data then we aren't truly at the bottom, so we + // render the add row component at the top + if (hasNextPage) { + return 0 + } + offset = rowCount * rowHeight - (scrollTop % rowHeight) + if (rowCount !== 0) { + offset -= 1 + } + return offset + } const addRow = async () => { // Blur the active cell and tick to let final value updates propagate @@ -89,12 +105,6 @@ return } - // If we have a next page of data then we aren't truly at the bottom, so we - // render the add row component at the top - if ($hasNextPage) { - offset = 0 - } - // If we don't have a next page then we're at the bottom and can scroll to // the max available offset else { @@ -102,10 +112,6 @@ ...state, top: $maxScrollTop, })) - offset = $renderedRows.length * $rowHeight - ($maxScrollTop % $rowHeight) - if ($renderedRows.length !== 0) { - offset -= 1 - } } // Update state and select initial cell @@ -175,39 +181,41 @@ {#if visible}
0} style="--offset:{offset}px; --sticky-width:{width}px;" >
- - - {#if isAdding} -
- {/if} - - {#if $displayColumn} - {@const cellId = getCellID(NewRowID, $displayColumn.name)} - - {#if $displayColumn?.schema?.autocolumn} -
Can't edit auto column
- {/if} +
+ + {#if isAdding}
{/if} - - {/if} + + {#if $displayColumn} + {@const cellId = getCellID(NewRowID, $displayColumn.name)} + + {#if $displayColumn?.schema?.autocolumn} +
Can't edit auto column
+ {/if} + {#if isAdding} +
+ {/if} + + {/if} +
@@ -274,7 +282,7 @@ margin-left: -6px; } - .container { + .new-row { position: absolute; top: var(--default-row-height); left: 0; @@ -284,10 +292,10 @@ flex-direction: row; align-items: stretch; } - .container :global(.cell) { + .new-row :global(.cell) { --cell-background: var(--spectrum-global-color-gray-75) !important; } - .container.floating :global(.cell) { + .new-row.floating :global(.cell) { height: calc(var(--row-height) + 1px); border-top: var(--cell-border); } @@ -316,8 +324,10 @@ pointer-events: all; z-index: 3; position: absolute; - top: calc(var(--row-height) + var(--offset) + 24px); - left: 18px; + top: calc( + var(--row-height) + var(--offset) + var(--default-row-height) / 2 + ); + left: calc(var(--default-row-height) / 2); } .button-with-keys { display: flex; diff --git a/packages/frontend-core/src/components/grid/layout/StickyColumn.svelte b/packages/frontend-core/src/components/grid/layout/StickyColumn.svelte index c971e98521..f240f964fa 100644 --- a/packages/frontend-core/src/components/grid/layout/StickyColumn.svelte +++ b/packages/frontend-core/src/components/grid/layout/StickyColumn.svelte @@ -69,66 +69,62 @@ -
- - {#each $renderedRows as row, idx} - {@const rowSelected = !!$selectedRows[row._id]} - {@const rowHovered = - $hoveredRowId === row._id && - (!$selectedCellCount || !$isSelectingCells)} - {@const rowFocused = $focusedRow?._id === row._id} - {@const cellId = getCellID(row._id, $displayColumn?.name)} -
($hoveredRowId = row._id)} - on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} - on:click={() => dispatch("rowclick", rows.actions.cleanRow(row))} - > - - {#if $displayColumn} - - {/if} -
- {/each} - {#if $config.canAddRows} -
($hoveredRowId = BlankRowID)} - on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} - on:click={() => dispatch("add-row-inline")} - > - - - - {#if $displayColumn} - - - - {/if} -
- {/if} -
-
+ + {#each $renderedRows as row, idx} + {@const rowSelected = !!$selectedRows[row._id]} + {@const rowHovered = + $hoveredRowId === row._id && + (!$selectedCellCount || !$isSelectingCells)} + {@const rowFocused = $focusedRow?._id === row._id} + {@const cellId = getCellID(row._id, $displayColumn?.name)} +
($hoveredRowId = row._id)} + on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} + on:click={() => dispatch("rowclick", rows.actions.cleanRow(row))} + > + + {#if $displayColumn} + + {/if} +
+ {/each} + {#if $config.canAddRows} +
($hoveredRowId = BlankRowID)} + on:mouseleave={$isDragging ? null : () => ($hoveredRowId = null)} + on:click={() => dispatch("add-row-inline")} + > + + + + {#if $displayColumn} + + + + {/if} +
+ {/if} +
diff --git a/packages/frontend-core/src/components/grid/lib/constants.js b/packages/frontend-core/src/components/grid/lib/constants.js index 4b5d04894a..6ea7a98178 100644 --- a/packages/frontend-core/src/components/grid/lib/constants.js +++ b/packages/frontend-core/src/components/grid/lib/constants.js @@ -1,12 +1,13 @@ -export const Padding = 100 -export const ScrollBarSize = 8 -export const GutterWidth = 72 -export const DefaultColumnWidth = 200 -export const MinColumnWidth = 80 export const SmallRowHeight = 36 export const MediumRowHeight = 64 export const LargeRowHeight = 92 export const DefaultRowHeight = SmallRowHeight +export const VPadding = SmallRowHeight * 2 +export const HPadding = 40 +export const ScrollBarSize = 8 +export const GutterWidth = 72 +export const DefaultColumnWidth = 200 +export const MinColumnWidth = 80 export const NewRowID = "new" export const BlankRowID = "blank" export const RowPageSize = 100 diff --git a/packages/frontend-core/src/components/grid/stores/scroll.js b/packages/frontend-core/src/components/grid/stores/scroll.js index 38809137c6..88d166ac9d 100644 --- a/packages/frontend-core/src/components/grid/stores/scroll.js +++ b/packages/frontend-core/src/components/grid/stores/scroll.js @@ -1,6 +1,12 @@ import { writable, derived, get } from "svelte/store" import { tick } from "svelte" -import { Padding, GutterWidth, FocusedCellMinOffset } from "../lib/constants" +import { + GutterWidth, + FocusedCellMinOffset, + ScrollBarSize, + HPadding, + VPadding, +} from "../lib/constants" import { parseCellID } from "../lib/utils" export const createStores = () => { @@ -36,26 +42,15 @@ export const deriveStores = context => { return ($displayColumn?.width || 0) + GutterWidth }) - // Derive vertical limits - const contentHeight = derived( - [rows, rowHeight], - ([$rows, $rowHeight]) => ($rows.length + 1) * $rowHeight + Padding - ) - const maxScrollTop = derived( - [height, contentHeight], - ([$height, $contentHeight]) => Math.max($contentHeight - $height, 0) - ) - // Derive horizontal limits const contentWidth = derived( [visibleColumns, buttonColumnWidth], ([$visibleColumns, $buttonColumnWidth]) => { - const space = Math.max(Padding, $buttonColumnWidth - 1) - let width = GutterWidth + space + let width = GutterWidth + $buttonColumnWidth $visibleColumns.forEach(col => { width += col.width }) - return width + return width + HPadding } ) const screenWidth = derived( @@ -70,14 +65,6 @@ export const deriveStores = context => { return Math.max($contentWidth - $screenWidth, 0) } ) - - // Derive whether to show scrollbars or not - const showVScrollbar = derived( - [contentHeight, height], - ([$contentHeight, $height]) => { - return $contentHeight > $height - } - ) const showHScrollbar = derived( [contentWidth, screenWidth], ([$contentWidth, $screenWidth]) => { @@ -85,6 +72,28 @@ export const deriveStores = context => { } ) + // Derive vertical limits + const contentHeight = derived( + [rows, rowHeight, showHScrollbar], + ([$rows, $rowHeight, $showHScrollbar]) => { + let height = ($rows.length + 1) * $rowHeight + VPadding + if ($showHScrollbar) { + height += ScrollBarSize * 2 + } + return height + } + ) + const maxScrollTop = derived( + [height, contentHeight], + ([$height, $contentHeight]) => Math.max($contentHeight - $height, 0) + ) + const showVScrollbar = derived( + [contentHeight, height], + ([$contentHeight, $height]) => { + return $contentHeight > $height + } + ) + return { stickyWidth, contentHeight, diff --git a/packages/server/src/api/routes/tests/table.spec.ts b/packages/server/src/api/routes/tests/table.spec.ts index f23e0de6db..8102966ad1 100644 --- a/packages/server/src/api/routes/tests/table.spec.ts +++ b/packages/server/src/api/routes/tests/table.spec.ts @@ -276,6 +276,31 @@ describe.each([ }) }) + isInternal && + it("shouldn't allow duplicate column names", async () => { + const saveTableRequest: SaveTableRequest = { + ...basicTable(), + } + saveTableRequest.schema["Type"] = { + type: FieldType.STRING, + name: "Type", + } + // allow the "Type" column - internal columns aren't case sensitive + await config.api.table.save(saveTableRequest, { + status: 200, + }) + saveTableRequest.schema.foo = { type: FieldType.STRING, name: "foo" } + saveTableRequest.schema.FOO = { type: FieldType.STRING, name: "FOO" } + + await config.api.table.save(saveTableRequest, { + status: 400, + body: { + message: + 'Column(s) "foo" are duplicated - check for other columns with these name (case in-sensitive)', + }, + }) + }) + it("should add a new column for an internal DB table", async () => { const saveTableRequest: SaveTableRequest = { ...basicTable(), diff --git a/packages/server/src/sdk/app/tables/internal/index.ts b/packages/server/src/sdk/app/tables/internal/index.ts index ea40d2bfe9..fc32708708 100644 --- a/packages/server/src/sdk/app/tables/internal/index.ts +++ b/packages/server/src/sdk/app/tables/internal/index.ts @@ -17,6 +17,7 @@ import { cloneDeep } from "lodash/fp" import isEqual from "lodash/isEqual" import { runStaticFormulaChecks } from "../../../../api/controllers/table/bulkFormula" import { context } from "@budibase/backend-core" +import { findDuplicateInternalColumns } from "@budibase/shared-core" import { getTable } from "../getters" import { checkAutoColumns } from "./utils" import * as viewsSdk from "../../views" @@ -44,6 +45,17 @@ export async function save( if (hasTypeChanged(table, oldTable)) { throw new Error("A column type has changed.") } + + // check for case sensitivity - we don't want to allow duplicated columns + const duplicateColumn = findDuplicateInternalColumns(table) + if (duplicateColumn.length) { + throw new Error( + `Column(s) "${duplicateColumn.join( + ", " + )}" are duplicated - check for other columns with these name (case in-sensitive)` + ) + } + // check that subtypes have been maintained table = checkAutoColumns(table, oldTable) diff --git a/packages/shared-core/src/constants/index.ts b/packages/shared-core/src/constants/index.ts index 82ac0d51c3..c9d1a8fc8f 100644 --- a/packages/shared-core/src/constants/index.ts +++ b/packages/shared-core/src/constants/index.ts @@ -1,5 +1,6 @@ export * from "./api" export * from "./fields" +export * from "./rows" export const OperatorOptions = { Equals: { diff --git a/packages/shared-core/src/constants/rows.ts b/packages/shared-core/src/constants/rows.ts new file mode 100644 index 0000000000..bfa7595d62 --- /dev/null +++ b/packages/shared-core/src/constants/rows.ts @@ -0,0 +1,14 @@ +export const CONSTANT_INTERNAL_ROW_COLS = [ + "_id", + "_rev", + "type", + "createdAt", + "updatedAt", + "tableId", +] as const + +export const CONSTANT_EXTERNAL_ROW_COLS = ["_id", "_rev", "tableId"] as const + +export function isInternalColumnName(name: string): boolean { + return (CONSTANT_INTERNAL_ROW_COLS as readonly string[]).includes(name) +} diff --git a/packages/shared-core/src/table.ts b/packages/shared-core/src/table.ts index 7706b78037..8fd7909b18 100644 --- a/packages/shared-core/src/table.ts +++ b/packages/shared-core/src/table.ts @@ -1,4 +1,5 @@ -import { FieldType } from "@budibase/types" +import { FieldType, Table } from "@budibase/types" +import { CONSTANT_INTERNAL_ROW_COLS } from "./constants" const allowDisplayColumnByType: Record = { [FieldType.STRING]: true, @@ -51,3 +52,27 @@ export function canBeDisplayColumn(type: FieldType): boolean { export function canBeSortColumn(type: FieldType): boolean { return !!allowSortColumnByType[type] } + +export function findDuplicateInternalColumns(table: Table): string[] { + // maintains the case of keys + const casedKeys = Object.keys(table.schema) + // get the column names + const uncasedKeys = casedKeys.map(colName => colName.toLowerCase()) + // there are duplicates + const set = new Set(uncasedKeys) + let duplicates: string[] = [] + if (set.size !== uncasedKeys.length) { + for (let key of set.keys()) { + const count = uncasedKeys.filter(name => name === key).length + if (count > 1) { + duplicates.push(key) + } + } + } + for (let internalColumn of CONSTANT_INTERNAL_ROW_COLS) { + if (casedKeys.find(key => key === internalColumn)) { + duplicates.push(internalColumn) + } + } + return duplicates +}