From e88ffea1a4262579bfdb7e1fcf6f0e179542bcd4 Mon Sep 17 00:00:00 2001
From: Gerard Burns <Ghrehh@users.noreply.github.com>
Date: Tue, 18 Jun 2024 11:18:05 +0100
Subject: [PATCH 1/2] Modal component (#13848)
* wip
* wip
* wip
* wip
* wip
* add note for illegalChildren reset behavior
* on close working
* wip
* lint
* wip
* Fix potential remounting loop caused by spreading props and unnecessary component keying
* theme
* user prompt
* dotted border for empty
* PR Feedback
* lint
* fix modal background color
* use bbui modal
* lint
* fix indicator and prevent closing modal in builder
* pr feedback
* pr feedback
* fix fullscreen
---------
Co-authored-by: deanhannigan <deanhannigan@gmail.com>
Co-authored-by: Andrew Kingston <andrew@kingston.dev>
---
packages/bbui/src/Modal/Modal.svelte | 2 +-
.../actions/CloseModal.svelte | 8 +
.../actions/OpenModal.svelte | 36 +++++
.../ButtonActionEditor/actions/index.js | 2 +
.../controls/ButtonActionEditor/manifest.json | 12 ++
.../new/_components/NewComponentPanel.svelte | 9 +-
.../new/_components/componentStructure.json | 2 +-
.../builder/src/stores/builder/screens.js | 9 +-
packages/client/manifest.json | 49 +++++-
.../client/src/components/ClientApp.svelte | 10 +-
.../client/src/components/app/Layout.svelte | 3 +
.../client/src/components/app/Link.svelte | 9 +-
.../client/src/components/app/Modal.svelte | 141 ++++++++++++++++++
.../app/blocks/form/FormBlock.svelte | 63 ++++----
.../app/blocks/form/InnerFormBlock.svelte | 16 +-
packages/client/src/components/app/index.js | 1 +
.../components/preview/IndicatorSet.svelte | 5 +-
packages/client/src/sdk.js | 2 +
packages/client/src/stores/index.js | 1 +
packages/client/src/stores/modal.js | 32 ++++
packages/client/src/utils/buttonActions.js | 14 ++
packages/frontend-core/src/utils/utils.js | 3 +
22 files changed, 376 insertions(+), 53 deletions(-)
create mode 100644 packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/CloseModal.svelte
create mode 100644 packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/OpenModal.svelte
create mode 100644 packages/client/src/components/app/Modal.svelte
create mode 100644 packages/client/src/stores/modal.js
diff --git a/packages/bbui/src/Modal/Modal.svelte b/packages/bbui/src/Modal/Modal.svelte
index 4656be69d1..dec1455d0c 100644
--- a/packages/bbui/src/Modal/Modal.svelte
+++ b/packages/bbui/src/Modal/Modal.svelte
@@ -162,6 +162,7 @@
max-height: 100%;
}
.modal-inner-wrapper {
+ padding: 40px;
flex: 1 1 auto;
display: flex;
flex-direction: row;
@@ -176,7 +177,6 @@
border: 2px solid var(--spectrum-global-color-gray-200);
overflow: visible;
max-height: none;
- margin: 40px 0;
transform: none;
--spectrum-dialog-confirm-border-radius: var(
--spectrum-global-dimension-size-100
diff --git a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/CloseModal.svelte b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/CloseModal.svelte
new file mode 100644
index 0000000000..ed0ca2c72b
--- /dev/null
+++ b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/CloseModal.svelte
@@ -0,0 +1,8 @@
+<div class="root">This action doesn't require any settings.</div>
+
+<style>
+ .root {
+ max-width: 400px;
+ margin: 0 auto;
+ }
+</style>
diff --git a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/OpenModal.svelte b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/OpenModal.svelte
new file mode 100644
index 0000000000..8e61b8763f
--- /dev/null
+++ b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/OpenModal.svelte
@@ -0,0 +1,36 @@
+<script>
+ import { Select, Label } from "@budibase/bbui"
+ import { selectedScreen } from "stores/builder"
+ import { findAllMatchingComponents } from "helpers/components"
+
+ export let parameters
+
+ $: modalOptions = getModalOptions($selectedScreen)
+
+ const getModalOptions = screen => {
+ const modalComponents = findAllMatchingComponents(screen.props, component =>
+ component._component.endsWith("/modal")
+ )
+ return modalComponents.map(modal => ({
+ label: modal._instanceName,
+ value: modal._id,
+ }))
+ }
+</script>
+
+<div class="root">
+ <Label small>Modal</Label>
+ <Select bind:value={parameters.id} options={modalOptions} />
+</div>
+
+<style>
+ .root {
+ display: grid;
+ column-gap: var(--spacing-l);
+ row-gap: var(--spacing-s);
+ grid-template-columns: 60px 1fr;
+ align-items: center;
+ max-width: 400px;
+ margin: 0 auto;
+ }
+</style>
diff --git a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/index.js b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/index.js
index 587993377d..606ee41d02 100644
--- a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/index.js
+++ b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/actions/index.js
@@ -21,5 +21,7 @@ export { default as ShowNotification } from "./ShowNotification.svelte"
export { default as PromptUser } from "./PromptUser.svelte"
export { default as OpenSidePanel } from "./OpenSidePanel.svelte"
export { default as CloseSidePanel } from "./CloseSidePanel.svelte"
+export { default as OpenModal } from "./OpenModal.svelte"
+export { default as CloseModal } from "./CloseModal.svelte"
export { default as ClearRowSelection } from "./ClearRowSelection.svelte"
export { default as DownloadFile } from "./DownloadFile.svelte"
diff --git a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/manifest.json b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/manifest.json
index 2840a0d662..4022926e7f 100644
--- a/packages/builder/src/components/design/settings/controls/ButtonActionEditor/manifest.json
+++ b/packages/builder/src/components/design/settings/controls/ButtonActionEditor/manifest.json
@@ -157,6 +157,18 @@
"component": "CloseSidePanel",
"dependsOnFeature": "sidePanel"
},
+ {
+ "name": "Open Modal",
+ "type": "application",
+ "component": "OpenModal",
+ "dependsOnFeature": "modal"
+ },
+ {
+ "name": "Close Modal",
+ "type": "application",
+ "component": "CloseModal",
+ "dependsOnFeature": "modal"
+ },
{
"name": "Clear Row Selection",
"type": "data",
diff --git a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/NewComponentPanel.svelte b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/NewComponentPanel.svelte
index c7c58a6e16..361e07a026 100644
--- a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/NewComponentPanel.svelte
+++ b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/NewComponentPanel.svelte
@@ -59,7 +59,14 @@
// Build up list of illegal children from ancestors
let illegalChildren = definition.illegalChildren || []
path.forEach(ancestor => {
- if (ancestor._component === `@budibase/standard-components/sidepanel`) {
+ // Sidepanels and modals can be nested anywhere in the component tree, but really they are always rendered at the top level.
+ // Because of this, it doesn't make sense to carry over any parent illegal children to them, so the array is reset here.
+ if (
+ [
+ "@budibase/standard-components/sidepanel",
+ "@budibase/standard-components/modal",
+ ].includes(ancestor._component)
+ ) {
illegalChildren = []
}
const def = componentStore.getDefinition(ancestor._component)
diff --git a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/componentStructure.json b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/componentStructure.json
index ba6f403d81..ff58a66221 100644
--- a/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/componentStructure.json
+++ b/packages/builder/src/pages/builder/app/[application]/design/[screenId]/[componentId]/new/_components/componentStructure.json
@@ -14,7 +14,7 @@
{
"name": "Layout",
"icon": "ClassicGridView",
- "children": ["container", "section", "sidepanel"]
+ "children": ["container", "section", "sidepanel", "modal"]
},
{
"name": "Data",
diff --git a/packages/builder/src/stores/builder/screens.js b/packages/builder/src/stores/builder/screens.js
index 7339593960..b1bef10c36 100644
--- a/packages/builder/src/stores/builder/screens.js
+++ b/packages/builder/src/stores/builder/screens.js
@@ -125,7 +125,14 @@ export class ScreenStore extends BudiStore {
return
}
- if (type === "@budibase/standard-components/sidepanel") {
+ // Sidepanels and modals can be nested anywhere in the component tree, but really they are always rendered at the top level.
+ // Because of this, it doesn't make sense to carry over any parent illegal children to them, so the array is reset here.
+ if (
+ [
+ "@budibase/standard-components/sidepanel",
+ "@budibase/standard-components/modal",
+ ].includes(type)
+ ) {
illegalChildren = []
}
diff --git a/packages/client/manifest.json b/packages/client/manifest.json
index 38e9bd8a87..00b503626f 100644
--- a/packages/client/manifest.json
+++ b/packages/client/manifest.json
@@ -11,6 +11,7 @@
"continueIfAction": true,
"showNotificationAction": true,
"sidePanel": true,
+ "modal": true,
"skeletonLoader": true
},
"typeSupportPresets": {
@@ -6975,7 +6976,7 @@
"name": "Side Panel",
"icon": "RailRight",
"hasChildren": true,
- "illegalChildren": ["section", "sidepanel"],
+ "illegalChildren": ["section", "sidepanel", "modal"],
"showEmptyState": false,
"draggable": false,
"info": "Side panels are hidden by default. They will only be revealed when triggered by the 'Open Side Panel' action.",
@@ -6993,6 +6994,52 @@
}
]
},
+ "modal": {
+ "name": "Modal",
+ "icon": "MBox",
+ "hasChildren": true,
+ "illegalChildren": ["section", "modal", "sidepanel"],
+ "showEmptyState": false,
+ "draggable": false,
+ "info": "Modals are hidden by default. They will only be revealed when triggered by the 'Open Modal' action.",
+ "settings": [
+ {
+ "type": "boolean",
+ "key": "ignoreClicksOutside",
+ "label": "Ignore clicks outside",
+ "defaultValue": false
+ },
+ {
+ "type": "event",
+ "key": "onClose",
+ "label": "On close"
+ },
+ {
+ "type": "select",
+ "label": "Size",
+ "key": "size",
+ "defaultValue": "small",
+ "options": [
+ {
+ "label": "Small",
+ "value": "small"
+ },
+ {
+ "label": "Medium",
+ "value": "medium"
+ },
+ {
+ "label": "Large",
+ "value": "large"
+ },
+ {
+ "label": "Fullscreen",
+ "value": "fullscreen"
+ }
+ ]
+ }
+ ]
+ },
"rowexplorer": {
"block": true,
"name": "Row Explorer Block",
diff --git a/packages/client/src/components/ClientApp.svelte b/packages/client/src/components/ClientApp.svelte
index c1bdc92ac4..9bfb1192ea 100644
--- a/packages/client/src/components/ClientApp.svelte
+++ b/packages/client/src/components/ClientApp.svelte
@@ -20,6 +20,7 @@
devToolsEnabled,
environmentStore,
sidePanelStore,
+ modalStore,
} from "stores"
import NotificationDisplay from "components/overlay/NotificationDisplay.svelte"
import ConfirmationDisplay from "components/overlay/ConfirmationDisplay.svelte"
@@ -104,10 +105,15 @@
})
}
const handleHashChange = () => {
- const { open } = $sidePanelStore
- if (open) {
+ const { open: sidePanelOpen } = $sidePanelStore
+ if (sidePanelOpen) {
sidePanelStore.actions.close()
}
+
+ const { open: modalOpen } = $modalStore
+ if (modalOpen) {
+ modalStore.actions.close()
+ }
}
window.addEventListener("hashchange", handleHashChange)
return () => {
diff --git a/packages/client/src/components/app/Layout.svelte b/packages/client/src/components/app/Layout.svelte
index 72da3e9012..af74e14aa0 100644
--- a/packages/client/src/components/app/Layout.svelte
+++ b/packages/client/src/components/app/Layout.svelte
@@ -12,6 +12,7 @@
linkable,
builderStore,
sidePanelStore,
+ modalStore,
appStore,
} = sdk
const context = getContext("context")
@@ -77,6 +78,7 @@
!$builderStore.inBuilder &&
$sidePanelStore.open &&
!$sidePanelStore.ignoreClicksOutside
+
$: screenId = $builderStore.inBuilder
? `${$builderStore.screen?._id}-screen`
: "screen"
@@ -198,6 +200,7 @@
const handleClickLink = () => {
mobileOpen = false
sidePanelStore.actions.close()
+ modalStore.actions.close()
}
</script>
diff --git a/packages/client/src/components/app/Link.svelte b/packages/client/src/components/app/Link.svelte
index 7eddcc6fe5..1ddc63066d 100644
--- a/packages/client/src/components/app/Link.svelte
+++ b/packages/client/src/components/app/Link.svelte
@@ -1,7 +1,7 @@
<script>
import { getContext } from "svelte"
- const { linkable, styleable, builderStore, sidePanelStore } =
+ const { linkable, styleable, builderStore, sidePanelStore, modalStore } =
getContext("sdk")
const component = getContext("component")
@@ -29,6 +29,11 @@
// overrides the color when it's passed as inline style.
$: styles = enrichStyles($component.styles, color)
+ const handleUrlChange = () => {
+ sidePanelStore.actions.close()
+ modalStore.actions.close()
+ }
+
const getSanitizedUrl = (url, externalLink, newTab) => {
if (!url) {
return externalLink || newTab ? "#/" : "/"
@@ -109,7 +114,7 @@
class:italic
class:underline
class="align--{align || 'left'} size--{size || 'M'}"
- on:click={sidePanelStore.actions.close}
+ on:click={handleUrlChange}
>
{componentText}
</a>
diff --git a/packages/client/src/components/app/Modal.svelte b/packages/client/src/components/app/Modal.svelte
new file mode 100644
index 0000000000..5c60735959
--- /dev/null
+++ b/packages/client/src/components/app/Modal.svelte
@@ -0,0 +1,141 @@
+<script>
+ import { getContext } from "svelte"
+ import { Modal, Icon } from "@budibase/bbui"
+
+ const component = getContext("component")
+ const { styleable, modalStore, builderStore, dndIsDragging } =
+ getContext("sdk")
+
+ export let onClose
+ export let ignoreClicksOutside
+ export let size
+ let modal
+
+ // Open modal automatically in builder
+ $: {
+ if ($builderStore.inBuilder) {
+ if (
+ $component.inSelectedPath &&
+ $modalStore.contentId !== $component.id
+ ) {
+ modalStore.actions.open($component.id)
+ } else if (
+ !$component.inSelectedPath &&
+ $modalStore.contentId === $component.id &&
+ !$dndIsDragging
+ ) {
+ modalStore.actions.close()
+ }
+ }
+ }
+
+ $: open = $modalStore.contentId === $component.id
+
+ const handleModalClose = async () => {
+ if (onClose) {
+ await onClose()
+ }
+ modalStore.actions.close()
+ }
+
+ const handleOpen = (open, modal) => {
+ if (!modal) return
+
+ if (open) {
+ modal.show()
+ } else {
+ modal.hide()
+ }
+ }
+
+ $: handleOpen(open, modal)
+</script>
+
+<!-- Conditional displaying in the builder is necessary otherwise previews don't update properly upon component deletion -->
+{#if !$builderStore.inBuilder || open}
+ <Modal
+ on:cancel={handleModalClose}
+ bind:this={modal}
+ disableCancel={$builderStore.inBuilder}
+ zIndex={2}
+ >
+ <div use:styleable={$component.styles} class={`modal-content ${size}`}>
+ <div class="modal-header">
+ <Icon
+ color="var(--spectrum-global-color-gray-800)"
+ name="Close"
+ hoverable
+ on:click={handleModalClose}
+ />
+ </div>
+ <div class="modal-main">
+ <div class="modal-main-inner">
+ <slot />
+ </div>
+ </div>
+ </div>
+ </Modal>
+{/if}
+
+<style>
+ .modal-content {
+ display: flex;
+ flex-direction: column;
+ max-width: 100%;
+ box-sizing: border-box;
+ padding: 12px 0px 40px;
+ }
+
+ .small {
+ width: 400px;
+ min-height: 200px;
+ }
+
+ .medium {
+ width: 600px;
+ min-height: 400px;
+ }
+
+ .large {
+ width: 800px;
+ min-height: 600px;
+ }
+
+ .fullscreen {
+ width: calc(100vw - 80px);
+ min-height: calc(100vh - 80px);
+ }
+
+ .modal-header {
+ display: flex;
+ flex-direction: row;
+ justify-content: flex-end;
+ flex-shrink: 0;
+ flex-grow: 0;
+ padding: 0 12px 12px;
+ box-sizing: border-box;
+ }
+
+ .modal-main {
+ padding: 0 40px;
+ flex-grow: 1;
+ display: flex;
+ flex-direction: column;
+ }
+
+ .modal-main :global(.component > *) {
+ max-width: 100%;
+ }
+
+ .modal-main-inner {
+ flex-grow: 1;
+ display: flex;
+ flex-direction: column;
+ word-break: break-word;
+ }
+
+ .modal-main-inner:empty {
+ border-radius: 3px;
+ border: 2px dashed var(--spectrum-global-color-gray-400);
+ }
+</style>
diff --git a/packages/client/src/components/app/blocks/form/FormBlock.svelte b/packages/client/src/components/app/blocks/form/FormBlock.svelte
index d249569731..e3aa20ffa6 100644
--- a/packages/client/src/components/app/blocks/form/FormBlock.svelte
+++ b/packages/client/src/components/app/blocks/form/FormBlock.svelte
@@ -31,41 +31,23 @@
let schema
- $: formattedFields = convertOldFieldFormat(fields)
- $: fieldsOrDefault = getDefaultFields(formattedFields, schema)
$: fetchSchema(dataSource)
$: id = $component.id
- // We could simply spread $$props into the inner form and append our
- // additions, but that would create svelte warnings about unused props and
- // make maintenance in future more confusing as we typically always have a
- // proper mapping of schema settings to component exports, without having to
- // search multiple files
- $: innerProps = {
- dataSource,
- actionUrl,
- actionType,
- size,
- disabled,
- fields: fieldsOrDefault,
- title,
- description,
- schema,
- notificationOverride,
- buttons:
- buttons ||
- Utils.buildFormBlockButtonConfig({
- _id: id,
- showDeleteButton,
- showSaveButton,
- saveButtonLabel,
- deleteButtonLabel,
- notificationOverride,
- actionType,
- actionUrl,
- dataSource,
- }),
- buttonPosition: buttons ? buttonPosition : "top",
- }
+ $: formattedFields = convertOldFieldFormat(fields)
+ $: fieldsOrDefault = getDefaultFields(formattedFields, schema)
+ $: buttonsOrDefault =
+ buttons ||
+ Utils.buildFormBlockButtonConfig({
+ _id: id,
+ showDeleteButton,
+ showSaveButton,
+ saveButtonLabel,
+ deleteButtonLabel,
+ notificationOverride,
+ actionType,
+ actionUrl,
+ dataSource,
+ })
// Provide additional data context for live binding eval
export const getAdditionalDataContext = () => {
@@ -123,5 +105,18 @@
</script>
<FormBlockWrapper {actionType} {dataSource} {rowId} {noRowsMessage}>
- <InnerFormBlock {...innerProps} />
+ <InnerFormBlock
+ {dataSource}
+ {actionUrl}
+ {actionType}
+ {size}
+ {disabled}
+ fields={fieldsOrDefault}
+ {title}
+ {description}
+ {schema}
+ {notificationOverride}
+ buttons={buttonsOrDefault}
+ buttonPosition={buttons ? buttonPosition : "top"}
+ />
</FormBlockWrapper>
diff --git a/packages/client/src/components/app/blocks/form/InnerFormBlock.svelte b/packages/client/src/components/app/blocks/form/InnerFormBlock.svelte
index b0733f3f4b..0227107dd2 100644
--- a/packages/client/src/components/app/blocks/form/InnerFormBlock.svelte
+++ b/packages/client/src/components/app/blocks/form/InnerFormBlock.svelte
@@ -91,15 +91,13 @@
{#if description}
<BlockComponent type="text" props={{ text: description }} order={1} />
{/if}
- {#key fields}
- <BlockComponent type="container">
- <div class="form-block fields" class:mobile={$context.device.mobile}>
- {#each fields as field, idx}
- <FormBlockComponent {field} {schema} order={idx} />
- {/each}
- </div>
- </BlockComponent>
- {/key}
+ <BlockComponent type="container">
+ <div class="form-block fields" class:mobile={$context.device.mobile}>
+ {#each fields as field, idx}
+ <FormBlockComponent {field} {schema} order={idx} />
+ {/each}
+ </div>
+ </BlockComponent>
</BlockComponent>
{#if buttonPosition === "bottom"}
<BlockComponent
diff --git a/packages/client/src/components/app/index.js b/packages/client/src/components/app/index.js
index e23e19704c..6d9df6e588 100644
--- a/packages/client/src/components/app/index.js
+++ b/packages/client/src/components/app/index.js
@@ -37,6 +37,7 @@ export { default as markdownviewer } from "./MarkdownViewer.svelte"
export { default as embeddedmap } from "./embedded-map/EmbeddedMap.svelte"
export { default as grid } from "./Grid.svelte"
export { default as sidepanel } from "./SidePanel.svelte"
+export { default as modal } from "./Modal.svelte"
export { default as gridblock } from "./GridBlock.svelte"
export * from "./charts"
export * from "./forms"
diff --git a/packages/client/src/components/preview/IndicatorSet.svelte b/packages/client/src/components/preview/IndicatorSet.svelte
index 3cbd7e2464..2b941b2662 100644
--- a/packages/client/src/components/preview/IndicatorSet.svelte
+++ b/packages/client/src/components/preview/IndicatorSet.svelte
@@ -57,7 +57,9 @@
return
}
nextState.indicators[idx].visible =
- nextState.indicators[idx].insideSidePanel || entries[0].isIntersecting
+ nextState.indicators[idx].insideModal ||
+ nextState.indicators[idx].insideSidePanel ||
+ entries[0].isIntersecting
if (++callbackCount === observers.length) {
state = nextState
updating = false
@@ -139,6 +141,7 @@
height: elBounds.height + 4,
visible: false,
insideSidePanel: !!child.closest(".side-panel"),
+ insideModal: !!child.closest(".modal-content"),
})
})
}
diff --git a/packages/client/src/sdk.js b/packages/client/src/sdk.js
index 90e0f9c7dc..50d3f857d5 100644
--- a/packages/client/src/sdk.js
+++ b/packages/client/src/sdk.js
@@ -11,6 +11,7 @@ import {
currentRole,
environmentStore,
sidePanelStore,
+ modalStore,
dndIsDragging,
confirmationStore,
roleStore,
@@ -53,6 +54,7 @@ export default {
componentStore,
environmentStore,
sidePanelStore,
+ modalStore,
dndIsDragging,
currentRole,
confirmationStore,
diff --git a/packages/client/src/stores/index.js b/packages/client/src/stores/index.js
index e9b1ce4434..f2b80ed732 100644
--- a/packages/client/src/stores/index.js
+++ b/packages/client/src/stores/index.js
@@ -27,6 +27,7 @@ export {
dndIsDragging,
} from "./dnd"
export { sidePanelStore } from "./sidePanel"
+export { modalStore } from "./modal"
export { hoverStore } from "./hover"
// Context stores are layered and duplicated, so it is not a singleton
diff --git a/packages/client/src/stores/modal.js b/packages/client/src/stores/modal.js
new file mode 100644
index 0000000000..4d1331283d
--- /dev/null
+++ b/packages/client/src/stores/modal.js
@@ -0,0 +1,32 @@
+import { writable } from "svelte/store"
+
+export const createModalStore = () => {
+ const initialState = {
+ contentId: null,
+ }
+ const store = writable(initialState)
+
+ const open = id => {
+ store.update(state => {
+ state.contentId = id
+ return state
+ })
+ }
+
+ const close = () => {
+ store.update(state => {
+ state.contentId = null
+ return state
+ })
+ }
+
+ return {
+ subscribe: store.subscribe,
+ actions: {
+ open,
+ close,
+ },
+ }
+}
+
+export const modalStore = createModalStore()
diff --git a/packages/client/src/utils/buttonActions.js b/packages/client/src/utils/buttonActions.js
index 482b36cdb8..bd220b8e85 100644
--- a/packages/client/src/utils/buttonActions.js
+++ b/packages/client/src/utils/buttonActions.js
@@ -12,6 +12,7 @@ import {
uploadStore,
rowSelectionStore,
sidePanelStore,
+ modalStore,
} from "stores"
import { API } from "api"
import { ActionTypes } from "constants"
@@ -436,6 +437,17 @@ const closeSidePanelHandler = () => {
sidePanelStore.actions.close()
}
+const openModalHandler = action => {
+ const { id } = action.parameters
+ if (id) {
+ modalStore.actions.open(id)
+ }
+}
+
+const closeModalHandler = () => {
+ modalStore.actions.close()
+}
+
const downloadFileHandler = async action => {
const { url, fileName } = action.parameters
try {
@@ -499,6 +511,8 @@ const handlerMap = {
["Prompt User"]: promptUserHandler,
["Open Side Panel"]: openSidePanelHandler,
["Close Side Panel"]: closeSidePanelHandler,
+ ["Open Modal"]: openModalHandler,
+ ["Close Modal"]: closeModalHandler,
["Download File"]: downloadFileHandler,
}
diff --git a/packages/frontend-core/src/utils/utils.js b/packages/frontend-core/src/utils/utils.js
index 65690cd535..1bee3d6c04 100644
--- a/packages/frontend-core/src/utils/utils.js
+++ b/packages/frontend-core/src/utils/utils.js
@@ -161,6 +161,9 @@ export const buildFormBlockButtonConfig = props => {
{
"##eventHandlerType": "Close Side Panel",
},
+ {
+ "##eventHandlerType": "Close Modal",
+ },
// Clear a create form once submitted
...(actionType !== "Create"
? []
From 2b96cbcad79dab09de3cbc1626bd3d52094cf09b Mon Sep 17 00:00:00 2001
From: Peter Clement <PClmnt@users.noreply.github.com>
Date: Tue, 18 Jun 2024 13:45:58 +0100
Subject: [PATCH 2/2] Expose old row binding in automations (#13931)
* expose old row through the emitter
* accidentally added oldRow to step
* fix row fetch in external datasources
* add test for new / old row comparison
* add testing for old row update event
* allow function overloading in test files
* update tests per comments
* handle event race condition
* update test data modal to account for old row output
* switch icon positioning
---
.eslintrc.json | 3 +-
.../SetupPanel/AutomationBlockSetup.svelte | 75 +++++++++++++++----
.../src/api/controllers/row/external.ts | 4 +-
.../server/src/api/controllers/row/index.ts | 4 +-
.../src/api/controllers/row/internal.ts | 6 +-
.../src/api/routes/tests/automation.spec.ts | 43 ++++++++++-
.../server/src/api/routes/tests/row.spec.ts | 49 ++++++++++++
.../routes/tests/utilities/TestFunctions.ts | 11 +--
.../src/automations/triggerInfo/rowUpdated.ts | 9 ++-
packages/server/src/automations/triggers.ts | 10 ++-
packages/server/src/events/BudibaseEmitter.ts | 10 ++-
packages/server/src/events/utils.ts | 4 +
.../server/src/tests/utilities/structures.ts | 30 ++++++++
.../types/src/documents/app/automation.ts | 9 +++
14 files changed, 234 insertions(+), 33 deletions(-)
diff --git a/.eslintrc.json b/.eslintrc.json
index 9dab2f1a88..f614f1ad91 100644
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -92,7 +92,8 @@
// differs to external, but the API is broadly the same
"jest/no-conditional-expect": "off",
// have to turn this off to allow function overloading in typescript
- "no-dupe-class-members": "off"
+ "no-dupe-class-members": "off",
+ "no-redeclare": "off"
}
},
{
diff --git a/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte b/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte
index b8b7c5ae54..57ca19ddb2 100644
--- a/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte
+++ b/packages/builder/src/components/automation/SetupPanel/AutomationBlockSetup.svelte
@@ -16,6 +16,8 @@
DatePicker,
DrawerContent,
Toggle,
+ Icon,
+ Divider,
} from "@budibase/bbui"
import CreateWebhookModal from "components/automation/Shared/CreateWebhookModal.svelte"
import { automationStore, selectedAutomation, tables } from "stores/builder"
@@ -89,6 +91,8 @@
? [hbAutocomplete([...bindingsToCompletions(bindings, codeMode)])]
: []
+ let testDataRowVisibility = {}
+
const getInputData = (testData, blockInputs) => {
// Test data is not cloned for reactivity
let newInputData = testData || cloneDeep(blockInputs)
@@ -196,7 +200,8 @@
(automation.trigger?.event === "row:update" ||
automation.trigger?.event === "row:save")
) {
- if (name !== "id" && name !== "revision") return `trigger.row.${name}`
+ let noRowKeywordBindings = ["id", "revision", "oldRow"]
+ if (!noRowKeywordBindings.includes(name)) return `trigger.row.${name}`
}
/* End special cases for generating custom schemas based on triggers */
@@ -372,7 +377,11 @@
function getFieldLabel(key, value) {
const requiredSuffix = requiredProperties.includes(key) ? "*" : ""
- return `${value.title || (key === "row" ? "Table" : key)} ${requiredSuffix}`
+ return `${value.title || (key === "row" ? "Row" : key)} ${requiredSuffix}`
+ }
+
+ function toggleTestDataRowVisibility(key) {
+ testDataRowVisibility[key] = !testDataRowVisibility[key]
}
function handleAttachmentParams(keyValueObj) {
@@ -607,20 +616,48 @@
on:change={e => onChange(e, key)}
/>
{:else if value.customType === "row"}
- <RowSelector
- value={inputData[key]}
- meta={inputData["meta"] || {}}
- on:change={e => {
- if (e.detail?.key) {
- onChange(e, e.detail.key)
- } else {
- onChange(e, key)
- }
- }}
- {bindings}
- {isTestModal}
- {isUpdateRow}
- />
+ {#if isTestModal}
+ <div class="align-horizontally">
+ <Icon
+ name={testDataRowVisibility[key] ? "Remove" : "Add"}
+ hoverable
+ on:click={() => toggleTestDataRowVisibility(key)}
+ />
+ <Label size="XL">{label}</Label>
+ </div>
+ {#if testDataRowVisibility[key]}
+ <RowSelector
+ value={inputData[key]}
+ meta={inputData["meta"] || {}}
+ on:change={e => {
+ if (e.detail?.key) {
+ onChange(e, e.detail.key)
+ } else {
+ onChange(e, key)
+ }
+ }}
+ {bindings}
+ {isTestModal}
+ {isUpdateRow}
+ />
+ {/if}
+ <Divider />
+ {:else}
+ <RowSelector
+ value={inputData[key]}
+ meta={inputData["meta"] || {}}
+ on:change={e => {
+ if (e.detail?.key) {
+ onChange(e, e.detail.key)
+ } else {
+ onChange(e, key)
+ }
+ }}
+ {bindings}
+ {isTestModal}
+ {isUpdateRow}
+ />
+ {/if}
{:else if value.customType === "webhookUrl"}
<WebhookDisplay
on:change={e => onChange(e, key)}
@@ -736,6 +773,12 @@
width: 320px;
}
+ .align-horizontally {
+ display: flex;
+ gap: var(--spacing-s);
+ align-items: center;
+ }
+
.fields {
display: flex;
flex-direction: column;
diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts
index d301155231..5b12b5c207 100644
--- a/packages/server/src/api/controllers/row/external.ts
+++ b/packages/server/src/api/controllers/row/external.ts
@@ -39,9 +39,10 @@ export async function handleRequest<T extends Operation>(
export async function patch(ctx: UserCtx<PatchRowRequest, PatchRowResponse>) {
const tableId = utils.getTableId(ctx)
- const { _id, ...rowData } = ctx.request.body
+ const { _id, ...rowData } = ctx.request.body
const table = await sdk.tables.getTable(tableId)
+
const { row: dataToUpdate } = await inputProcessing(
ctx.user?._id,
cloneDeep(table),
@@ -79,6 +80,7 @@ export async function patch(ctx: UserCtx<PatchRowRequest, PatchRowResponse>) {
...response,
row: enrichedRow,
table,
+ oldRow: beforeRow,
}
}
diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts
index 945b7ca847..760b73f404 100644
--- a/packages/server/src/api/controllers/row/index.ts
+++ b/packages/server/src/api/controllers/row/index.ts
@@ -55,13 +55,13 @@ export async function patch(
return save(ctx)
}
try {
- const { row, table } = await pickApi(tableId).patch(ctx)
+ const { row, table, oldRow } = await pickApi(tableId).patch(ctx)
if (!row) {
ctx.throw(404, "Row not found")
}
ctx.status = 200
ctx.eventEmitter &&
- ctx.eventEmitter.emitRow(`row:update`, appId, row, table)
+ ctx.eventEmitter.emitRow(`row:update`, appId, row, table, oldRow)
ctx.message = `${table.name} updated successfully.`
ctx.body = row
gridSocket?.emitRowUpdate(ctx, row)
diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts
index cc903bd74a..54d9b6a536 100644
--- a/packages/server/src/api/controllers/row/internal.ts
+++ b/packages/server/src/api/controllers/row/internal.ts
@@ -85,13 +85,15 @@ export async function patch(ctx: UserCtx<PatchRowRequest, PatchRowResponse>) {
// the row has been updated, need to put it into the ctx
ctx.request.body = row as any
await userController.updateMetadata(ctx as any)
- return { row: ctx.body as Row, table }
+ return { row: ctx.body as Row, table, oldRow }
}
- return finaliseRow(table, row, {
+ const result = await finaliseRow(table, row, {
oldTable: dbTable,
updateFormula: true,
})
+
+ return { ...result, oldRow }
}
export async function find(ctx: UserCtx): Promise<Row> {
diff --git a/packages/server/src/api/routes/tests/automation.spec.ts b/packages/server/src/api/routes/tests/automation.spec.ts
index 711cfb8d4f..8cbd14d8b3 100644
--- a/packages/server/src/api/routes/tests/automation.spec.ts
+++ b/packages/server/src/api/routes/tests/automation.spec.ts
@@ -13,6 +13,7 @@ import { events } from "@budibase/backend-core"
import sdk from "../../../sdk"
import { Automation } from "@budibase/types"
import { mocks } from "@budibase/backend-core/tests"
+import { FilterConditions } from "../../../automations/steps/filter"
const MAX_RETRIES = 4
let {
@@ -21,6 +22,7 @@ let {
automationTrigger,
automationStep,
collectAutomation,
+ filterAutomation,
} = setup.structures
describe("/automations", () => {
@@ -155,7 +157,12 @@ describe("/automations", () => {
automation.appId = config.appId
automation = await config.createAutomation(automation)
await setup.delay(500)
- const res = await testAutomation(config, automation)
+ const res = await testAutomation(config, automation, {
+ row: {
+ name: "Test",
+ description: "TEST",
+ },
+ })
expect(events.automation.tested).toHaveBeenCalledTimes(1)
// this looks a bit mad but we don't actually have a way to wait for a response from the automation to
// know that it has finished all of its actions - this is currently the best way
@@ -436,4 +443,38 @@ describe("/automations", () => {
expect(res).toEqual(true)
})
})
+
+ describe("Update Row Old / New Row comparison", () => {
+ it.each([
+ { oldCity: "asdsadsadsad", newCity: "new" },
+ { oldCity: "Belfast", newCity: "Belfast" },
+ ])(
+ "triggers an update row automation and compares new to old rows with old city '%s' and new city '%s'",
+ async ({ oldCity, newCity }) => {
+ const expectedResult = oldCity === newCity
+
+ let table = await config.createTable()
+
+ let automation = await filterAutomation()
+ automation.definition.trigger.inputs.tableId = table._id
+ automation.definition.steps[0].inputs = {
+ condition: FilterConditions.EQUAL,
+ field: "{{ trigger.row.City }}",
+ value: "{{ trigger.oldRow.City }}",
+ }
+ automation.appId = config.appId!
+ automation = await config.createAutomation(automation)
+ let triggerInputs = {
+ oldRow: {
+ City: oldCity,
+ },
+ row: {
+ City: newCity,
+ },
+ }
+ const res = await testAutomation(config, automation, triggerInputs)
+ expect(res.body.steps[1].outputs.result).toEqual(expectedResult)
+ }
+ )
+ })
})
diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts
index f822615a87..e5599c6d81 100644
--- a/packages/server/src/api/routes/tests/row.spec.ts
+++ b/packages/server/src/api/routes/tests/row.spec.ts
@@ -1,6 +1,7 @@
import { DatabaseName, getDatasource } from "../../../integrations/tests/utils"
import tk from "timekeeper"
+import emitter from "../../../../src/events"
import { outputProcessing } from "../../../utilities/rowProcessor"
import * as setup from "./utilities"
import { context, InternalTable, tenancy } from "@budibase/backend-core"
@@ -24,6 +25,7 @@ import {
StaticQuotaName,
Table,
TableSourceType,
+ UpdatedRowEventEmitter,
} from "@budibase/types"
import { generator, mocks } from "@budibase/backend-core/tests"
import _, { merge } from "lodash"
@@ -31,6 +33,28 @@ import * as uuid from "uuid"
const timestamp = new Date("2023-01-26T11:48:57.597Z").toISOString()
tk.freeze(timestamp)
+interface WaitOptions {
+ name: string
+ matchFn?: (event: any) => boolean
+}
+async function waitForEvent(
+ opts: WaitOptions,
+ callback: () => Promise<void>
+): Promise<any> {
+ const p = new Promise((resolve: any) => {
+ const listener = (event: any) => {
+ if (opts.matchFn && !opts.matchFn(event)) {
+ return
+ }
+ resolve(event)
+ emitter.off(opts.name, listener)
+ }
+ emitter.on(opts.name, listener)
+ })
+
+ await callback()
+ return await p
+}
describe.each([
["internal", undefined],
@@ -608,6 +632,31 @@ describe.each([
await assertRowUsage(rowUsage)
})
+ it("should update only the fields that are supplied and emit the correct oldRow", async () => {
+ let beforeRow = await config.api.row.save(table._id!, {
+ name: "test",
+ description: "test",
+ })
+ const opts = {
+ name: "row:update",
+ matchFn: (event: UpdatedRowEventEmitter) =>
+ event.row._id === beforeRow._id,
+ }
+ const event = await waitForEvent(opts, async () => {
+ await config.api.row.patch(table._id!, {
+ _id: beforeRow._id!,
+ _rev: beforeRow._rev!,
+ tableId: table._id!,
+ name: "Updated Name",
+ })
+ })
+
+ expect(event.oldRow).toBeDefined()
+ expect(event.oldRow.name).toEqual("test")
+ expect(event.row.name).toEqual("Updated Name")
+ expect(event.oldRow.description).toEqual(beforeRow.description)
+ expect(event.row.description).toEqual(beforeRow.description)
+ })
it("should throw an error when given improper types", async () => {
const existing = await config.api.row.save(table._id!, {})
const rowUsage = await getRowUsage()
diff --git a/packages/server/src/api/routes/tests/utilities/TestFunctions.ts b/packages/server/src/api/routes/tests/utilities/TestFunctions.ts
index 8a843551ac..27d8592849 100644
--- a/packages/server/src/api/routes/tests/utilities/TestFunctions.ts
+++ b/packages/server/src/api/routes/tests/utilities/TestFunctions.ts
@@ -158,15 +158,16 @@ export const getDB = () => {
return context.getAppDB()
}
-export const testAutomation = async (config: any, automation: any) => {
+export const testAutomation = async (
+ config: any,
+ automation: any,
+ triggerInputs: any
+) => {
return runRequest(automation.appId, async () => {
return await config.request
.post(`/api/automations/${automation._id}/test`)
.send({
- row: {
- name: "Test",
- description: "TEST",
- },
+ ...triggerInputs,
})
.set(config.defaultHeaders())
.expect("Content-Type", /json/)
diff --git a/packages/server/src/automations/triggerInfo/rowUpdated.ts b/packages/server/src/automations/triggerInfo/rowUpdated.ts
index 5e60015808..eab7c40a09 100644
--- a/packages/server/src/automations/triggerInfo/rowUpdated.ts
+++ b/packages/server/src/automations/triggerInfo/rowUpdated.ts
@@ -27,10 +27,17 @@ export const definition: AutomationTriggerSchema = {
},
outputs: {
properties: {
- row: {
+ oldRow: {
type: AutomationIOType.OBJECT,
customType: AutomationCustomIOType.ROW,
description: "The row that was updated",
+ title: "Old Row",
+ },
+ row: {
+ type: AutomationIOType.OBJECT,
+ customType: AutomationCustomIOType.ROW,
+ description: "The row before it was updated",
+ title: "Row",
},
id: {
type: AutomationIOType.STRING,
diff --git a/packages/server/src/automations/triggers.ts b/packages/server/src/automations/triggers.ts
index 223b8d2eb6..9aa80035bd 100644
--- a/packages/server/src/automations/triggers.ts
+++ b/packages/server/src/automations/triggers.ts
@@ -8,7 +8,13 @@ import { checkTestFlag } from "../utilities/redis"
import * as utils from "./utils"
import env from "../environment"
import { context, db as dbCore } from "@budibase/backend-core"
-import { Automation, Row, AutomationData, AutomationJob } from "@budibase/types"
+import {
+ Automation,
+ Row,
+ AutomationData,
+ AutomationJob,
+ UpdatedRowEventEmitter,
+} from "@budibase/types"
import { executeInThread } from "../threads/automation"
export const TRIGGER_DEFINITIONS = definitions
@@ -65,7 +71,7 @@ async function queueRelevantRowAutomations(
})
}
-emitter.on("row:save", async function (event) {
+emitter.on("row:save", async function (event: UpdatedRowEventEmitter) {
/* istanbul ignore next */
if (!event || !event.row || !event.row.tableId) {
return
diff --git a/packages/server/src/events/BudibaseEmitter.ts b/packages/server/src/events/BudibaseEmitter.ts
index 43871d8754..8feb36bbf5 100644
--- a/packages/server/src/events/BudibaseEmitter.ts
+++ b/packages/server/src/events/BudibaseEmitter.ts
@@ -13,8 +13,14 @@ import { Table, Row } from "@budibase/types"
* This is specifically quite important for template strings used in automations.
*/
class BudibaseEmitter extends EventEmitter {
- emitRow(eventName: string, appId: string, row: Row, table?: Table) {
- rowEmission({ emitter: this, eventName, appId, row, table })
+ emitRow(
+ eventName: string,
+ appId: string,
+ row: Row,
+ table?: Table,
+ oldRow?: Row
+ ) {
+ rowEmission({ emitter: this, eventName, appId, row, table, oldRow })
}
emitTable(eventName: string, appId: string, table?: Table) {
diff --git a/packages/server/src/events/utils.ts b/packages/server/src/events/utils.ts
index 20efb453f2..b972c8e473 100644
--- a/packages/server/src/events/utils.ts
+++ b/packages/server/src/events/utils.ts
@@ -7,6 +7,7 @@ type BBEventOpts = {
appId: string
table?: Table
row?: Row
+ oldRow?: Row
metadata?: any
}
@@ -18,6 +19,7 @@ type BBEvent = {
appId: string
tableId?: string
row?: Row
+ oldRow?: Row
table?: BBEventTable
id?: string
revision?: string
@@ -31,9 +33,11 @@ export function rowEmission({
row,
table,
metadata,
+ oldRow,
}: BBEventOpts) {
let event: BBEvent = {
row,
+ oldRow,
appId,
tableId: row?.tableId,
}
diff --git a/packages/server/src/tests/utilities/structures.ts b/packages/server/src/tests/utilities/structures.ts
index 7213cc66f1..a59719ab2c 100644
--- a/packages/server/src/tests/utilities/structures.ts
+++ b/packages/server/src/tests/utilities/structures.ts
@@ -359,6 +359,36 @@ export function collectAutomation(tableId?: string): Automation {
return automation as Automation
}
+export function filterAutomation(tableId?: string): Automation {
+ const automation: any = {
+ name: "looping",
+ type: "automation",
+ definition: {
+ steps: [
+ {
+ id: "b",
+ type: "ACTION",
+ internal: true,
+ stepId: AutomationActionStepId.FILTER,
+ inputs: {},
+ schema: BUILTIN_ACTION_DEFINITIONS.EXECUTE_SCRIPT.schema,
+ },
+ ],
+ trigger: {
+ id: "a",
+ type: "TRIGGER",
+ event: "row:save",
+ stepId: AutomationTriggerStepId.ROW_SAVED,
+ inputs: {
+ tableId,
+ },
+ schema: TRIGGER_DEFINITIONS.ROW_SAVED.schema,
+ },
+ },
+ }
+ return automation as Automation
+}
+
export function basicAutomationResults(
automationId: string
): AutomationResults {
diff --git a/packages/types/src/documents/app/automation.ts b/packages/types/src/documents/app/automation.ts
index 63291fa3bb..5954a47151 100644
--- a/packages/types/src/documents/app/automation.ts
+++ b/packages/types/src/documents/app/automation.ts
@@ -2,6 +2,8 @@ import { Document } from "../document"
import { EventEmitter } from "events"
import { User } from "../global"
import { ReadStream } from "fs"
+import { Row } from "./row"
+import { Table } from "./table"
export enum AutomationIOType {
OBJECT = "object",
@@ -252,3 +254,10 @@ export type BucketedContent = AutomationAttachmentContent & {
bucket: string
path: string
}
+
+export type UpdatedRowEventEmitter = {
+ row: Row
+ oldRow: Row
+ table: Table
+ appId: string
+}