From 2537ee6980524b142eee1c5fb2b66ea5bcf3cd65 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 13:13:11 +0100 Subject: [PATCH 01/17] Validate required settings messages from builder --- packages/builder/src/helpers/screen.ts | 2 +- .../src/stores/builder/screenComponent.ts | 142 ++++++++++++++---- .../client/src/components/Component.svelte | 6 +- .../error-states/ComponentErrorState.svelte | 2 +- 4 files changed, 114 insertions(+), 38 deletions(-) diff --git a/packages/builder/src/helpers/screen.ts b/packages/builder/src/helpers/screen.ts index 296a597adb..e9a478d059 100644 --- a/packages/builder/src/helpers/screen.ts +++ b/packages/builder/src/helpers/screen.ts @@ -38,7 +38,7 @@ export function findComponentsBySettingsType( return result } -function getManifestDefinition(component: Component) { +export function getManifestDefinition(component: Component) { const componentType = component._component.split("/").slice(-1)[0] const definition = clientManifest[componentType as keyof typeof clientManifest] diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index d8169fdedb..35e76464cf 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -2,11 +2,15 @@ import { derived } from "svelte/store" import { tables } from "./tables" import { selectedScreen } from "./screens" import { viewsV2 } from "./viewsV2" -import { findComponentsBySettingsType } from "@/helpers/screen" -import { UIDatasourceType, Screen } from "@budibase/types" +import { + findComponentsBySettingsType, + getManifestDefinition, +} from "@/helpers/screen" +import { UIDatasourceType, Screen, Component } from "@budibase/types" import { queries } from "./queries" import { views } from "./views" import { featureFlag } from "@/helpers" +import { findAllComponents } from "@/helpers/components" function reduceBy( key: TKey, @@ -42,34 +46,6 @@ export const screenComponentErrors = derived( if (!featureFlag.isEnabled("CHECK_SCREEN_COMPONENT_SETTINGS_ERRORS")) { return {} } - function getInvalidDatasources( - screen: Screen, - datasources: Record - ) { - const result: Record = {} - for (const { component, setting } of findComponentsBySettingsType( - screen, - ["table", "dataSource"] - )) { - const componentSettings = component[setting.key] - const { label } = componentSettings - const type = componentSettings.type as UIDatasourceType - - const validationKey = validationKeyByType[type] - if (!validationKey) { - continue - } - const resourceId = componentSettings[validationKey] - if (!datasources[resourceId]) { - const friendlyTypeName = friendlyNameByType[type] ?? type - result[component._id!] = [ - `The ${friendlyTypeName} named "${label}" could not be found`, - ] - } - } - - return result - } const datasources = { ...reduceBy("_id", $tables.list), @@ -78,6 +54,110 @@ export const screenComponentErrors = derived( ...reduceBy("_id", $queries.list), } - return getInvalidDatasources($selectedScreen, datasources) + const errors = { + ...getInvalidDatasources($selectedScreen, datasources), + ...getMissingRequiredSettings($selectedScreen), + } + return errors } ) + +function getInvalidDatasources( + screen: Screen, + datasources: Record +) { + const result: Record = {} + for (const { component, setting } of findComponentsBySettingsType(screen, [ + "table", + "dataSource", + ])) { + const componentSettings = component[setting.key] + if (!componentSettings) { + continue + } + + const { label } = componentSettings + const type = componentSettings.type as UIDatasourceType + + const validationKey = validationKeyByType[type] + if (!validationKey) { + continue + } + const resourceId = componentSettings[validationKey] + if (!datasources[resourceId]) { + const friendlyTypeName = friendlyNameByType[type] ?? type + result[component._id!] = [ + `The ${friendlyTypeName} named "${label}" could not be found`, + ] + } + } + + return result +} + +function getAllComponentsInScreen(screen: Screen) { + const result: Component[] = [] + function recursiveCheck(component: Component) { + result.push(...findAllComponents(component)) + component._children?.forEach(recursiveCheck) + } + recursiveCheck(screen.props) + return result +} + +function getMissingRequiredSettings(screen: Screen) { + const allComponents = getAllComponentsInScreen(screen) + + const result: Record = {} + for (const component of allComponents) { + const definition = getManifestDefinition(component) + if (!("settings" in definition)) { + continue + } + + const missingRequiredSettings = definition.settings.filter( + (setting: any) => { + let empty = + component[setting.key] == null || component[setting.key] === "" + let missing = setting.required && empty + + // Check if this setting depends on another, as it may not be required + if (setting.dependsOn) { + const dependsOnKey = setting.dependsOn.setting || setting.dependsOn + const dependsOnValue = setting.dependsOn.value + const realDependentValue = component[dependsOnKey] + + const sectionDependsOnKey = + setting.sectionDependsOn?.setting || setting.sectionDependsOn + const sectionDependsOnValue = setting.sectionDependsOn?.value + const sectionRealDependentValue = component[sectionDependsOnKey] + + if (dependsOnValue == null && realDependentValue == null) { + return false + } + if (dependsOnValue != null && dependsOnValue !== realDependentValue) { + return false + } + + if ( + sectionDependsOnValue != null && + sectionDependsOnValue !== sectionRealDependentValue + ) { + return false + } + } + + return missing + } + ) + + if (missingRequiredSettings.length) { + result[component._id!] = missingRequiredSettings.map( + (s: any) => + `Add the ${s.label} setting to start using your component` + ) + } + } + + return result +} diff --git a/packages/client/src/components/Component.svelte b/packages/client/src/components/Component.svelte index 236bcb7c7e..345603fe3a 100644 --- a/packages/client/src/components/Component.svelte +++ b/packages/client/src/components/Component.svelte @@ -695,11 +695,7 @@ use:gridLayout={gridMetadata} > {#if errorState} - + {:else} {#if children.length} diff --git a/packages/client/src/components/error-states/ComponentErrorState.svelte b/packages/client/src/components/error-states/ComponentErrorState.svelte index 7069b7a431..7292ffe55b 100644 --- a/packages/client/src/components/error-states/ComponentErrorState.svelte +++ b/packages/client/src/components/error-states/ComponentErrorState.svelte @@ -26,7 +26,7 @@ {#if requiredAncestor} {:else if errorMessage} - {errorMessage} + {@html errorMessage} {:else if requiredSetting} {/if} From 9f92f3414ae9c6b693496603b441dc8f24ac10f7 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 13:22:43 +0100 Subject: [PATCH 02/17] Add CTA on component error --- .../src/stores/builder/screenComponent.ts | 26 ++++++++++++------- .../error-states/ComponentErrorState.svelte | 18 +++++++++++-- packages/client/src/index.ts | 6 ++++- packages/types/src/ui/components/index.ts | 5 ++++ 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 35e76464cf..c2a41355dd 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -6,7 +6,12 @@ import { findComponentsBySettingsType, getManifestDefinition, } from "@/helpers/screen" -import { UIDatasourceType, Screen, Component } from "@budibase/types" +import { + UIDatasourceType, + Screen, + Component, + UIComponentError, +} from "@budibase/types" import { queries } from "./queries" import { views } from "./views" import { featureFlag } from "@/helpers" @@ -41,7 +46,7 @@ export const screenComponentErrors = derived( [selectedScreen, tables, views, viewsV2, queries], ([$selectedScreen, $tables, $views, $viewsV2, $queries]): Record< string, - string[] + UIComponentError[] > => { if (!featureFlag.isEnabled("CHECK_SCREEN_COMPONENT_SETTINGS_ERRORS")) { return {} @@ -66,7 +71,7 @@ function getInvalidDatasources( screen: Screen, datasources: Record ) { - const result: Record = {} + const result: Record = {} for (const { component, setting } of findComponentsBySettingsType(screen, [ "table", "dataSource", @@ -87,7 +92,10 @@ function getInvalidDatasources( if (!datasources[resourceId]) { const friendlyTypeName = friendlyNameByType[type] ?? type result[component._id!] = [ - `The ${friendlyTypeName} named "${label}" could not be found`, + { + key: setting.key, + message: `The ${friendlyTypeName} named "${label}" could not be found`, + }, ] } } @@ -108,7 +116,7 @@ function getAllComponentsInScreen(screen: Screen) { function getMissingRequiredSettings(screen: Screen) { const allComponents = getAllComponentsInScreen(screen) - const result: Record = {} + const result: Record = {} for (const component of allComponents) { const definition = getManifestDefinition(component) if (!("settings" in definition)) { @@ -152,10 +160,10 @@ function getMissingRequiredSettings(screen: Screen) { ) if (missingRequiredSettings.length) { - result[component._id!] = missingRequiredSettings.map( - (s: any) => - `Add the ${s.label} setting to start using your component` - ) + result[component._id!] = missingRequiredSettings.map((s: any) => ({ + key: s.key, + message: `Add the ${s.label} setting to start using your component`, + })) } } diff --git a/packages/client/src/components/error-states/ComponentErrorState.svelte b/packages/client/src/components/error-states/ComponentErrorState.svelte index 7292ffe55b..a4122054d7 100644 --- a/packages/client/src/components/error-states/ComponentErrorState.svelte +++ b/packages/client/src/components/error-states/ComponentErrorState.svelte @@ -3,12 +3,13 @@ import { Icon } from "@budibase/bbui" import MissingRequiredSetting from "./MissingRequiredSetting.svelte" import MissingRequiredAncestor from "./MissingRequiredAncestor.svelte" + import { UIComponentError } from "@budibase/types" export let missingRequiredSettings: | { key: string; label: string }[] | undefined export let missingRequiredAncestors: string[] | undefined - export let componentErrors: string[] | undefined + export let componentErrors: UIComponentError[] | undefined const component = getContext("component") const { styleable, builderStore } = getContext("sdk") @@ -26,7 +27,20 @@ {#if requiredAncestor} {:else if errorMessage} - {@html errorMessage} + {@html errorMessage.message} + {#if errorMessage.key} + - + + + { + builderStore.actions.highlightSetting(errorMessage.key) + }} + > + Show me + + {/if} {:else if requiredSetting} {/if} diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 2a435c2f8c..0e136f942c 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -11,7 +11,11 @@ export interface SDK { generateGoldenSample: any builderStore: Readable<{ inBuilder: boolean - }> + }> & { + actions: { + highlightSetting: (key: string) => void + } + } } export type Component = Readable<{ diff --git a/packages/types/src/ui/components/index.ts b/packages/types/src/ui/components/index.ts index 8dc1638f8c..5518842382 100644 --- a/packages/types/src/ui/components/index.ts +++ b/packages/types/src/ui/components/index.ts @@ -1,2 +1,7 @@ export * from "./sidepanel" export * from "./codeEditor" + +export interface UIComponentError { + key: string + message: string +} From a0fb2c8e593e699ff7e07cfe8559d01bccb2aa5b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 13:28:35 +0100 Subject: [PATCH 03/17] Fix after merge --- packages/builder/src/stores/builder/screenComponent.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 5ff7258bb2..841607e835 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -81,6 +81,10 @@ function getInvalidDatasources( "dataSource", ])) { const componentSettings = component[setting.key] + if (!componentSettings) { + continue + } + const { label } = componentSettings const type = componentSettings.type as UIDatasourceType From 224e12f7f3f471d05b1b1a8cc8a61738f2c5158e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 13:43:16 +0100 Subject: [PATCH 04/17] Validate ancestors --- packages/builder/src/helpers/screen.ts | 7 ++-- .../src/stores/builder/screenComponent.ts | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/packages/builder/src/helpers/screen.ts b/packages/builder/src/helpers/screen.ts index e9a478d059..24988c2195 100644 --- a/packages/builder/src/helpers/screen.ts +++ b/packages/builder/src/helpers/screen.ts @@ -38,8 +38,11 @@ export function findComponentsBySettingsType( return result } -export function getManifestDefinition(component: Component) { - const componentType = component._component.split("/").slice(-1)[0] +export function getManifestDefinition(component: Component | string) { + const componentType = + typeof component === "string" + ? component + : component._component.split("/").slice(-1)[0] const definition = clientManifest[componentType as keyof typeof clientManifest] return definition diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 841607e835..a94c2295df 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -65,6 +65,7 @@ export const screenComponentErrors = derived( const errors = { ...getInvalidDatasources($selectedScreen, datasources), + ...getMissingAncestors($selectedScreen), ...getMissingRequiredSettings($selectedScreen), } return errors @@ -182,3 +183,37 @@ function getMissingRequiredSettings(screen: Screen) { return result } + +const BudibasePrefix = "@budibase/standard-components/" +function getMissingAncestors(screen: Screen) { + const allComponents = getAllComponentsInScreen(screen) + const result: Record = {} + for (const component of allComponents) { + const definition = getManifestDefinition(component) + if (!("requiredAncestors" in definition)) { + continue + } + + const missingAncestors = definition.requiredAncestors.filter( + ancestor => !component.ancestors?.includes(`${BudibasePrefix}${ancestor}`) + ) + + if (missingAncestors.length) { + const pluralise = (name: string) => { + return name.endsWith("s") ? `${name}'` : `${name}s` + } + + const getAncestorName = (name: string) => { + const definition: any = getManifestDefinition(name) + return definition.name + } + + result[component._id!] = missingAncestors.map((s: any) => ({ + key: s.key, + message: `${pluralise(component._instanceName)} need to be inside a +${getAncestorName(s)}`, + })) + } + } + return result +} From 29d9e3d804443ed960768bc2a1735e314412bd37 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 13:49:58 +0100 Subject: [PATCH 05/17] Fix text --- packages/builder/src/stores/builder/screenComponent.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index a94c2295df..0a9bfadbc2 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -210,7 +210,7 @@ function getMissingAncestors(screen: Screen) { result[component._id!] = missingAncestors.map((s: any) => ({ key: s.key, - message: `${pluralise(component._instanceName)} need to be inside a + message: `${pluralise(definition.name)} need to be inside a ${getAncestorName(s)}`, })) } From 684cf27f0e32afad44198122edcea0cd82fcfc7f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 27 Jan 2025 16:32:14 +0100 Subject: [PATCH 06/17] Error CTA --- .../src/stores/builder/screenComponent.ts | 25 +++++++----- .../error-states/ComponentErrorState.svelte | 15 +------ .../ComponentErrorStateCTA.svelte | 40 +++++++++++++++++++ packages/client/src/index.ts | 4 ++ packages/types/src/ui/components/errors.ts | 20 ++++++++++ packages/types/src/ui/components/index.ts | 6 +-- 6 files changed, 82 insertions(+), 28 deletions(-) create mode 100644 packages/client/src/components/error-states/ComponentErrorStateCTA.svelte create mode 100644 packages/types/src/ui/components/errors.ts diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 76b69f6ec1..1cc295e0a8 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -112,6 +112,7 @@ function getInvalidDatasources( { key: setting.key, message: `The ${friendlyTypeName} named "${label}" could not be found`, + errorType: "setting", }, ] } @@ -180,6 +181,7 @@ function getMissingRequiredSettings(screen: Screen) { result[component._id!] = missingRequiredSettings.map((s: any) => ({ key: s.key, message: `Add the ${s.label} setting to start using your component`, + errorType: "setting", })) } } @@ -206,16 +208,19 @@ function getMissingAncestors(screen: Screen) { return name.endsWith("s") ? `${name}'` : `${name}s` } - const getAncestorName = (name: string) => { - const definition: any = getManifestDefinition(name) - return definition.name - } - - result[component._id!] = missingAncestors.map((s: any) => ({ - key: s.key, - message: `${pluralise(definition.name)} need to be inside a -${getAncestorName(s)}`, - })) + result[component._id!] = missingAncestors.map((ancestor: any) => { + const ancestorDefinition: any = getManifestDefinition(ancestor) + return { + key: ancestor.key, + message: `${pluralise(definition.name)} need to be inside a +${ancestorDefinition.name}`, + errorType: "ancestor-setting", + ancestor: { + name: ancestorDefinition.name, + fullType: `${BudibasePrefix}${ancestor}`, + }, + } + }) } } return result diff --git a/packages/client/src/components/error-states/ComponentErrorState.svelte b/packages/client/src/components/error-states/ComponentErrorState.svelte index a4122054d7..45740f4c2d 100644 --- a/packages/client/src/components/error-states/ComponentErrorState.svelte +++ b/packages/client/src/components/error-states/ComponentErrorState.svelte @@ -4,6 +4,7 @@ import MissingRequiredSetting from "./MissingRequiredSetting.svelte" import MissingRequiredAncestor from "./MissingRequiredAncestor.svelte" import { UIComponentError } from "@budibase/types" + import ComponentErrorStateCta from "./ComponentErrorStateCTA.svelte" export let missingRequiredSettings: | { key: string; label: string }[] @@ -28,19 +29,7 @@ {:else if errorMessage} {@html errorMessage.message} - {#if errorMessage.key} - - - - - { - builderStore.actions.highlightSetting(errorMessage.key) - }} - > - Show me - - {/if} + {:else if requiredSetting} {/if} diff --git a/packages/client/src/components/error-states/ComponentErrorStateCTA.svelte b/packages/client/src/components/error-states/ComponentErrorStateCTA.svelte new file mode 100644 index 0000000000..ef22f307f7 --- /dev/null +++ b/packages/client/src/components/error-states/ComponentErrorStateCTA.svelte @@ -0,0 +1,40 @@ + + +{#if error} + {#if error.errorType === "setting"} + - + + + { + builderStore.actions.highlightSetting(error.key) + }} + > + Show me + + {:else if error.errorType === "ancestor-setting"} + - + + + { + builderStore.actions.addParentComponent( + $component.id, + error.ancestor.fullType + ) + }} + > + Add {error.ancestor.name} + + {/if} +{/if} diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 0e136f942c..08330a60fa 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -14,6 +14,10 @@ export interface SDK { }> & { actions: { highlightSetting: (key: string) => void + addParentComponent: ( + componentId: string, + fullAncestorType: string + ) => void } } } diff --git a/packages/types/src/ui/components/errors.ts b/packages/types/src/ui/components/errors.ts new file mode 100644 index 0000000000..ce86e2f517 --- /dev/null +++ b/packages/types/src/ui/components/errors.ts @@ -0,0 +1,20 @@ +interface BaseUIComponentError { + key: string + message: string +} + +interface UISettingComponentError extends BaseUIComponentError { + errorType: "setting" +} + +interface UIAncestorComponentError extends BaseUIComponentError { + errorType: "ancestor-setting" + ancestor: { + name: string + fullType: string + } +} + +export type UIComponentError = + | UISettingComponentError + | UIAncestorComponentError diff --git a/packages/types/src/ui/components/index.ts b/packages/types/src/ui/components/index.ts index 5518842382..611f92b9a4 100644 --- a/packages/types/src/ui/components/index.ts +++ b/packages/types/src/ui/components/index.ts @@ -1,7 +1,3 @@ export * from "./sidepanel" export * from "./codeEditor" - -export interface UIComponentError { - key: string - message: string -} +export * from "./errors" From 582854cc2eaabe44fa2014dd1f03149daad50059 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 28 Jan 2025 09:59:53 +0100 Subject: [PATCH 07/17] Fix checking ancestors --- .../src/stores/builder/screenComponent.ts | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 1cc295e0a8..a2488145c3 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -122,13 +122,7 @@ function getInvalidDatasources( } function getAllComponentsInScreen(screen: Screen) { - const result: Component[] = [] - function recursiveCheck(component: Component) { - result.push(...findAllComponents(component)) - component._children?.forEach(recursiveCheck) - } - recursiveCheck(screen.props) - return result + return findAllComponents(screen.props) as Component[] } function getMissingRequiredSettings(screen: Screen) { @@ -191,16 +185,21 @@ function getMissingRequiredSettings(screen: Screen) { const BudibasePrefix = "@budibase/standard-components/" function getMissingAncestors(screen: Screen) { - const allComponents = getAllComponentsInScreen(screen) const result: Record = {} - for (const component of allComponents) { + + function checkMissingAncestors(component: Component, ancestors: string[]) { + for (const child of component._children || []) { + checkMissingAncestors(child, [...ancestors, component._component]) + } + const definition = getManifestDefinition(component) + if (!("requiredAncestors" in definition)) { - continue + return } const missingAncestors = definition.requiredAncestors.filter( - ancestor => !component.ancestors?.includes(`${BudibasePrefix}${ancestor}`) + ancestor => !ancestors.includes(`${BudibasePrefix}${ancestor}`) ) if (missingAncestors.length) { @@ -223,5 +222,7 @@ function getMissingAncestors(screen: Screen) { }) } } + + checkMissingAncestors(screen.props, []) return result } From 5b3bd1ad177c744304e34da860cd6dc933bb2478 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 28 Jan 2025 10:00:30 +0100 Subject: [PATCH 08/17] Clean --- packages/builder/src/stores/builder/screenComponent.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index a2488145c3..3284e9a339 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -121,12 +121,8 @@ function getInvalidDatasources( return result } -function getAllComponentsInScreen(screen: Screen) { - return findAllComponents(screen.props) as Component[] -} - function getMissingRequiredSettings(screen: Screen) { - const allComponents = getAllComponentsInScreen(screen) + const allComponents = findAllComponents(screen.props) as Component[] const result: Record = {} for (const component of allComponents) { From c793bd8f5ee73f4e3d3d79f07cceef23168d726a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 28 Jan 2025 11:07:40 +0100 Subject: [PATCH 09/17] Remove MissingRequired components from the client --- .../client/src/components/Component.svelte | 58 +++---------------- .../error-states/ComponentErrorState.svelte | 14 +---- .../MissingRequiredAncestor.svelte | 43 -------------- .../MissingRequiredSetting.svelte | 22 ------- 4 files changed, 8 insertions(+), 129 deletions(-) delete mode 100644 packages/client/src/components/error-states/MissingRequiredAncestor.svelte delete mode 100644 packages/client/src/components/error-states/MissingRequiredSetting.svelte diff --git a/packages/client/src/components/Component.svelte b/packages/client/src/components/Component.svelte index 345603fe3a..cbe93add57 100644 --- a/packages/client/src/components/Component.svelte +++ b/packages/client/src/components/Component.svelte @@ -29,7 +29,6 @@ import EmptyPlaceholder from "components/app/EmptyPlaceholder.svelte" import ScreenPlaceholder from "components/app/ScreenPlaceholder.svelte" import ComponentErrorState from "components/error-states/ComponentErrorState.svelte" - import { BudibasePrefix } from "../stores/components.js" import { decodeJSBinding, findHBSBlocks, @@ -102,7 +101,6 @@ let definition let settingsDefinition let settingsDefinitionMap - let missingRequiredSettings = false let componentErrors = false // Temporary styles which can be added in the app preview for things like @@ -141,18 +139,11 @@ $: componentErrors = instance?._meta?.errors $: hasChildren = !!definition?.hasChildren $: showEmptyState = definition?.showEmptyState !== false - $: hasMissingRequiredSettings = missingRequiredSettings?.length > 0 + $: hasMissingRequiredSettings = !!componentErrors?.find( + e => e.errorType === "setting" + ) $: editable = !!definition?.editable && !hasMissingRequiredSettings $: hasComponentErrors = componentErrors?.length > 0 - $: requiredAncestors = definition?.requiredAncestors || [] - $: missingRequiredAncestors = requiredAncestors.filter( - ancestor => !$component.ancestors.includes(`${BudibasePrefix}${ancestor}`) - ) - $: hasMissingRequiredAncestors = missingRequiredAncestors?.length > 0 - $: errorState = - hasMissingRequiredSettings || - hasMissingRequiredAncestors || - hasComponentErrors // Interactive components can be selected, dragged and highlighted inside // the builder preview @@ -218,7 +209,7 @@ styles: normalStyles, draggable, definition, - errored: errorState, + errored: hasComponentErrors, } // When dragging and dropping, pad components to allow dropping between @@ -251,9 +242,8 @@ name, editing, type: instance._component, - errorState, + errorState: hasComponentErrors, parent: id, - ancestors: [...($component?.ancestors ?? []), instance._component], path: [...($component?.path ?? []), id], darkMode, }) @@ -310,40 +300,6 @@ staticSettings = instanceSettings.staticSettings dynamicSettings = instanceSettings.dynamicSettings - // Check if we have any missing required settings - missingRequiredSettings = settingsDefinition.filter(setting => { - let empty = instance[setting.key] == null || instance[setting.key] === "" - let missing = setting.required && empty - - // Check if this setting depends on another, as it may not be required - if (setting.dependsOn) { - const dependsOnKey = setting.dependsOn.setting || setting.dependsOn - const dependsOnValue = setting.dependsOn.value - const realDependentValue = instance[dependsOnKey] - - const sectionDependsOnKey = - setting.sectionDependsOn?.setting || setting.sectionDependsOn - const sectionDependsOnValue = setting.sectionDependsOn?.value - const sectionRealDependentValue = instance[sectionDependsOnKey] - - if (dependsOnValue == null && realDependentValue == null) { - return false - } - if (dependsOnValue != null && dependsOnValue !== realDependentValue) { - return false - } - - if ( - sectionDependsOnValue != null && - sectionDependsOnValue !== sectionRealDependentValue - ) { - return false - } - } - - return missing - }) - // When considering bindings we can ignore children, so we remove that // before storing the reference stringified version const noChildren = JSON.stringify({ ...instance, _children: null }) @@ -686,7 +642,7 @@ class:pad class:parent={hasChildren} class:block={isBlock} - class:error={errorState} + class:error={hasComponentErrors} class:root={isRoot} data-id={id} data-name={name} @@ -694,7 +650,7 @@ data-parent={$component.id} use:gridLayout={gridMetadata} > - {#if errorState} + {#if hasComponentErrors} {:else} diff --git a/packages/client/src/components/error-states/ComponentErrorState.svelte b/packages/client/src/components/error-states/ComponentErrorState.svelte index 45740f4c2d..16c744b25f 100644 --- a/packages/client/src/components/error-states/ComponentErrorState.svelte +++ b/packages/client/src/components/error-states/ComponentErrorState.svelte @@ -1,23 +1,15 @@ @@ -25,13 +17,9 @@ {#if $component.errorState}
- {#if requiredAncestor} - - {:else if errorMessage} + {#if errorMessage} {@html errorMessage.message} - {:else if requiredSetting} - {/if}
{/if} diff --git a/packages/client/src/components/error-states/MissingRequiredAncestor.svelte b/packages/client/src/components/error-states/MissingRequiredAncestor.svelte deleted file mode 100644 index c251b1be4e..0000000000 --- a/packages/client/src/components/error-states/MissingRequiredAncestor.svelte +++ /dev/null @@ -1,43 +0,0 @@ - - - - {pluralName} need to be inside a - {ancestorName} - -- - - - { - builderStore.actions.addParentComponent($component.id, fullAncestorType) - }} -> - Add {ancestorName} - diff --git a/packages/client/src/components/error-states/MissingRequiredSetting.svelte b/packages/client/src/components/error-states/MissingRequiredSetting.svelte deleted file mode 100644 index 37e99a3cd1..0000000000 --- a/packages/client/src/components/error-states/MissingRequiredSetting.svelte +++ /dev/null @@ -1,22 +0,0 @@ - - - - Add the {requiredSetting.label} setting to start using your component - -- - - - { - builderStore.actions.highlightSetting(requiredSetting.key) - }} -> - Show me - From 59875b7d97b78c363f96e279f0061618741191b3 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 28 Jan 2025 11:24:38 +0100 Subject: [PATCH 10/17] Clean types --- packages/builder/src/stores/builder/screenComponent.ts | 3 +-- packages/types/src/ui/components/errors.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 3284e9a339..47ac11a2c4 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -203,10 +203,9 @@ function getMissingAncestors(screen: Screen) { return name.endsWith("s") ? `${name}'` : `${name}s` } - result[component._id!] = missingAncestors.map((ancestor: any) => { + result[component._id!] = missingAncestors.map(ancestor => { const ancestorDefinition: any = getManifestDefinition(ancestor) return { - key: ancestor.key, message: `${pluralise(definition.name)} need to be inside a ${ancestorDefinition.name}`, errorType: "ancestor-setting", diff --git a/packages/types/src/ui/components/errors.ts b/packages/types/src/ui/components/errors.ts index ce86e2f517..9725ed8a33 100644 --- a/packages/types/src/ui/components/errors.ts +++ b/packages/types/src/ui/components/errors.ts @@ -1,10 +1,10 @@ interface BaseUIComponentError { - key: string message: string } interface UISettingComponentError extends BaseUIComponentError { errorType: "setting" + key: string } interface UIAncestorComponentError extends BaseUIComponentError { From 232adee0a816931752618e1626cff995aefcec88 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 28 Jan 2025 11:39:17 +0100 Subject: [PATCH 11/17] Remove lint error --- .../src/components/error-states/ComponentErrorState.svelte | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/client/src/components/error-states/ComponentErrorState.svelte b/packages/client/src/components/error-states/ComponentErrorState.svelte index 16c744b25f..b2e7c92eae 100644 --- a/packages/client/src/components/error-states/ComponentErrorState.svelte +++ b/packages/client/src/components/error-states/ComponentErrorState.svelte @@ -18,6 +18,7 @@
{#if errorMessage} + {@html errorMessage.message} {/if} From f5fade510554fdd6a2dca83ae42daa1a977e9e8e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 28 Jan 2025 11:44:11 +0100 Subject: [PATCH 12/17] Clean --- packages/client/src/components/Component.svelte | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/client/src/components/Component.svelte b/packages/client/src/components/Component.svelte index cbe93add57..e023214083 100644 --- a/packages/client/src/components/Component.svelte +++ b/packages/client/src/components/Component.svelte @@ -101,7 +101,6 @@ let definition let settingsDefinition let settingsDefinitionMap - let componentErrors = false // Temporary styles which can be added in the app preview for things like // DND. We clear these whenever a new instance is received. From 141fc902c3c50c3c2809dcf1f494800b8294ec2f Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 31 Jan 2025 13:28:38 +0100 Subject: [PATCH 13/17] Remove unnecessary "in" checks --- packages/builder/src/stores/builder/screenComponent.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index 3338796283..de63c3fbb3 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -136,11 +136,8 @@ function getMissingRequiredSettings( const result: Record = {} for (const component of allComponents) { const definition = definitions[component._component] - if (!("settings" in definition)) { - continue - } - const missingRequiredSettings = definition.settings?.filter( + const missingRequiredSettings = definition?.settings?.filter( (setting: any) => { let empty = component[setting.key] == null || component[setting.key] === "" @@ -258,10 +255,10 @@ function findComponentsBySettingsType( const setting = definition?.settings?.find((s: any) => typesArray.includes(s.type) ) - if (setting && "type" in setting) { + if (setting) { result.push({ component, - setting: { type: setting.type!, key: setting.key! }, + setting: { type: setting.type, key: setting.key }, }) } component._children?.forEach(child => { From ea2b7b879154eab0f9520fce268fe06b1991509b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 31 Jan 2025 13:55:15 +0100 Subject: [PATCH 14/17] Move utils and types --- .../builder/src/stores/builder/components.ts | 26 ++-------------- packages/client/src/utils/componentProps.js | 23 -------------- .../frontend-core/src/utils/components.ts | 26 ++++++++++++++++ packages/frontend-core/src/utils/index.ts | 1 + packages/types/src/ui/components/index.ts | 30 +++++++++++++++++++ 5 files changed, 59 insertions(+), 47 deletions(-) create mode 100644 packages/frontend-core/src/utils/components.ts diff --git a/packages/builder/src/stores/builder/components.ts b/packages/builder/src/stores/builder/components.ts index f0dbc560c1..84ec963406 100644 --- a/packages/builder/src/stores/builder/components.ts +++ b/packages/builder/src/stores/builder/components.ts @@ -32,6 +32,8 @@ import { import { BudiStore } from "../BudiStore" import { Utils } from "@budibase/frontend-core" import { + ComponentDefinition, + ComponentSetting, Component as ComponentType, FieldType, Screen, @@ -53,30 +55,6 @@ export interface ComponentState { selectedScreenId?: string | null } -export interface ComponentDefinition { - component: string - name: string - friendlyName?: string - hasChildren?: boolean - settings?: ComponentSetting[] - features?: Record - typeSupportPresets?: Record - legalDirectChildren: string[] - requiredAncestors?: string[] - illegalChildren: string[] -} - -export interface ComponentSetting { - key: string - type: string - section?: string - name?: string - defaultValue?: any - selectAllFields?: boolean - resetOn?: string | string[] - settings?: ComponentSetting[] -} - export const INITIAL_COMPONENTS_STATE: ComponentState = { components: {}, customComponents: [], diff --git a/packages/client/src/utils/componentProps.js b/packages/client/src/utils/componentProps.js index bdf74c8014..eec284e3d8 100644 --- a/packages/client/src/utils/componentProps.js +++ b/packages/client/src/utils/componentProps.js @@ -97,26 +97,3 @@ export const propsUseBinding = (props, bindingKey) => { } return false } - -/** - * Gets the definition of this component's settings from the manifest - */ -export const getSettingsDefinition = definition => { - if (!definition) { - return [] - } - let settings = [] - definition.settings?.forEach(setting => { - if (setting.section) { - settings = settings.concat( - (setting.settings || [])?.map(childSetting => ({ - ...childSetting, - sectionDependsOn: setting.dependsOn, - })) - ) - } else { - settings.push(setting) - } - }) - return settings -} diff --git a/packages/frontend-core/src/utils/components.ts b/packages/frontend-core/src/utils/components.ts new file mode 100644 index 0000000000..60eb520258 --- /dev/null +++ b/packages/frontend-core/src/utils/components.ts @@ -0,0 +1,26 @@ +import { ComponentDefinition, ComponentSetting } from "@budibase/types" + +/** + * Gets the definition of this component's settings from the manifest + */ +export const getSettingsDefinition = ( + definition: ComponentDefinition +): ComponentSetting[] => { + if (!definition) { + return [] + } + let settings: ComponentSetting[] = [] + definition.settings?.forEach(setting => { + if (setting.section) { + settings = settings.concat( + (setting.settings || [])?.map(childSetting => ({ + ...childSetting, + sectionDependsOn: setting.dependsOn, + })) + ) + } else { + settings.push(setting) + } + }) + return settings +} diff --git a/packages/frontend-core/src/utils/index.ts b/packages/frontend-core/src/utils/index.ts index efc694d268..3fa9f66b22 100644 --- a/packages/frontend-core/src/utils/index.ts +++ b/packages/frontend-core/src/utils/index.ts @@ -13,3 +13,4 @@ export * from "./download" export * from "./settings" export * from "./relatedColumns" export * from "./table" +export * from "./components" diff --git a/packages/types/src/ui/components/index.ts b/packages/types/src/ui/components/index.ts index 611f92b9a4..61b0c6e564 100644 --- a/packages/types/src/ui/components/index.ts +++ b/packages/types/src/ui/components/index.ts @@ -1,3 +1,33 @@ export * from "./sidepanel" export * from "./codeEditor" export * from "./errors" + +export interface ComponentDefinition { + component: string + name: string + friendlyName?: string + hasChildren?: boolean + settings?: ComponentSetting[] + features?: Record + typeSupportPresets?: Record + legalDirectChildren: string[] + requiredAncestors?: string[] + illegalChildren: string[] +} + +export interface ComponentSetting { + key: string + type: string + section?: string + name?: string + defaultValue?: any + selectAllFields?: boolean + resetOn?: string | string[] + settings?: ComponentSetting[] + dependsOn?: + | string + | { + setting: string + value: string + } +} From 9976bcf125e45cbecb70fd8eb28467d195f1fc8c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 31 Jan 2025 13:55:27 +0100 Subject: [PATCH 15/17] Handle sections properly --- .../src/stores/builder/screenComponent.ts | 60 ++++++++++--------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/packages/builder/src/stores/builder/screenComponent.ts b/packages/builder/src/stores/builder/screenComponent.ts index de63c3fbb3..1d82924463 100644 --- a/packages/builder/src/stores/builder/screenComponent.ts +++ b/packages/builder/src/stores/builder/screenComponent.ts @@ -8,13 +8,15 @@ import { Component, UIComponentError, ScreenProps, + ComponentDefinition, } from "@budibase/types" import { queries } from "./queries" import { views } from "./views" import { findAllComponents } from "@/helpers/components" import { bindings, featureFlag } from "@/helpers" import { getBindableProperties } from "@/dataBinding" -import { componentStore, ComponentDefinition } from "./components" +import { componentStore } from "./components" +import { getSettingsDefinition } from "@budibase/frontend-core" function reduceBy( key: TKey, @@ -137,41 +139,41 @@ function getMissingRequiredSettings( for (const component of allComponents) { const definition = definitions[component._component] - const missingRequiredSettings = definition?.settings?.filter( - (setting: any) => { - let empty = - component[setting.key] == null || component[setting.key] === "" - let missing = setting.required && empty + const settings = getSettingsDefinition(definition) - // Check if this setting depends on another, as it may not be required - if (setting.dependsOn) { - const dependsOnKey = setting.dependsOn.setting || setting.dependsOn - const dependsOnValue = setting.dependsOn.value - const realDependentValue = component[dependsOnKey] + const missingRequiredSettings = settings.filter((setting: any) => { + let empty = + component[setting.key] == null || component[setting.key] === "" + let missing = setting.required && empty - const sectionDependsOnKey = - setting.sectionDependsOn?.setting || setting.sectionDependsOn - const sectionDependsOnValue = setting.sectionDependsOn?.value - const sectionRealDependentValue = component[sectionDependsOnKey] + // Check if this setting depends on another, as it may not be required + if (setting.dependsOn) { + const dependsOnKey = setting.dependsOn.setting || setting.dependsOn + const dependsOnValue = setting.dependsOn.value + const realDependentValue = component[dependsOnKey] - if (dependsOnValue == null && realDependentValue == null) { - return false - } - if (dependsOnValue != null && dependsOnValue !== realDependentValue) { - return false - } + const sectionDependsOnKey = + setting.sectionDependsOn?.setting || setting.sectionDependsOn + const sectionDependsOnValue = setting.sectionDependsOn?.value + const sectionRealDependentValue = component[sectionDependsOnKey] - if ( - sectionDependsOnValue != null && - sectionDependsOnValue !== sectionRealDependentValue - ) { - return false - } + if (dependsOnValue == null && realDependentValue == null) { + return false + } + if (dependsOnValue != null && dependsOnValue !== realDependentValue) { + return false } - return missing + if ( + sectionDependsOnValue != null && + sectionDependsOnValue !== sectionRealDependentValue + ) { + return false + } } - ) + + return missing + }) if (missingRequiredSettings?.length) { result[component._id!] = missingRequiredSettings.map((s: any) => ({ From 1274593cb20d81ae27b00eb74e0fdae30854bad2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Fri, 31 Jan 2025 13:57:37 +0100 Subject: [PATCH 16/17] Fix types --- packages/builder/src/stores/builder/screens.ts | 2 +- packages/client/src/components/Component.svelte | 7 ++----- .../devtools/DevToolsComponentSettingsTab.svelte | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/builder/src/stores/builder/screens.ts b/packages/builder/src/stores/builder/screens.ts index 64fe31752d..3f098334f0 100644 --- a/packages/builder/src/stores/builder/screens.ts +++ b/packages/builder/src/stores/builder/screens.ts @@ -19,8 +19,8 @@ import { Screen, Component, SaveScreenResponse, + ComponentDefinition, } from "@budibase/types" -import { ComponentDefinition } from "./components" interface ScreenState { screens: Screen[] diff --git a/packages/client/src/components/Component.svelte b/packages/client/src/components/Component.svelte index e023214083..3e22ffada1 100644 --- a/packages/client/src/components/Component.svelte +++ b/packages/client/src/components/Component.svelte @@ -11,11 +11,8 @@