From d89f5f74e50e3640296a8144e16160304dacb0f9 Mon Sep 17 00:00:00 2001 From: Andrew Kingston Date: Wed, 2 Mar 2022 19:10:18 +0000 Subject: [PATCH] Rewrite screen wizard to make modals reusable and fix some edge case URL bugs --- .../store/screenTemplates/index.js | 7 +- .../ComponentDropdownMenu.svelte | 2 - .../ScreenDropdownMenu.svelte | 5 + .../NavigationPanel/NewScreenModal.svelte | 48 ++--- .../NavigationPanel/ScreenDetailsModal.svelte | 43 ++-- .../NavigationPanel/ScreenWizard.svelte | 200 +++++++++--------- 6 files changed, 143 insertions(+), 162 deletions(-) diff --git a/packages/builder/src/builderStore/store/screenTemplates/index.js b/packages/builder/src/builderStore/store/screenTemplates/index.js index 38ae434753..51496bdeb3 100644 --- a/packages/builder/src/builderStore/store/screenTemplates/index.js +++ b/packages/builder/src/builderStore/store/screenTemplates/index.js @@ -10,17 +10,18 @@ const allTemplates = tables => [ ] // Allows us to apply common behaviour to all create() functions -const createTemplateOverride = (frontendState, create) => () => { - const screen = create() +const createTemplateOverride = (frontendState, template) => () => { + const screen = template.create() screen.name = screen.props._id screen.routing.route = screen.routing.route.toLowerCase() + screen.template = template.id return screen } export default (frontendState, tables) => { const enrichTemplate = template => ({ ...template, - create: createTemplateOverride(frontendState, template.create), + create: createTemplateOverride(frontendState, template), }) const fromScratch = enrichTemplate(createFromScratchScreen) diff --git a/packages/builder/src/components/design/NavigationPanel/ComponentDropdownMenu.svelte b/packages/builder/src/components/design/NavigationPanel/ComponentDropdownMenu.svelte index c8796fab25..df6be7e88f 100644 --- a/packages/builder/src/components/design/NavigationPanel/ComponentDropdownMenu.svelte +++ b/packages/builder/src/components/design/NavigationPanel/ComponentDropdownMenu.svelte @@ -73,13 +73,11 @@ } const storeComponentForCopy = (cut = false) => { - // lives in store - also used by drag drop store.actions.components.copy(component, cut) } const pasteComponent = mode => { try { - // lives in store - also used by drag drop store.actions.components.paste(component, mode) } catch (error) { notifications.error("Error saving component") diff --git a/packages/builder/src/components/design/NavigationPanel/ComponentNavigationTree/ScreenDropdownMenu.svelte b/packages/builder/src/components/design/NavigationPanel/ComponentNavigationTree/ScreenDropdownMenu.svelte index 38ed79649e..47c9fb65b1 100644 --- a/packages/builder/src/components/design/NavigationPanel/ComponentNavigationTree/ScreenDropdownMenu.svelte +++ b/packages/builder/src/components/design/NavigationPanel/ComponentNavigationTree/ScreenDropdownMenu.svelte @@ -10,6 +10,8 @@ $: screen = $allScreens.find(screen => screen._id === screenId) + const duplicateScreen = () => {} + const deleteScreen = async () => { try { await store.actions.screens.delete(screen) @@ -25,6 +27,9 @@
+ + Duplicate + Delete diff --git a/packages/builder/src/components/design/NavigationPanel/NewScreenModal.svelte b/packages/builder/src/components/design/NavigationPanel/NewScreenModal.svelte index c48ba33efa..cd83d81235 100644 --- a/packages/builder/src/components/design/NavigationPanel/NewScreenModal.svelte +++ b/packages/builder/src/components/design/NavigationPanel/NewScreenModal.svelte @@ -10,39 +10,19 @@ ProgressCircle, } from "@budibase/bbui" import getTemplates from "builderStore/store/screenTemplates" - import { onDestroy } from "svelte" - import { createEventDispatcher } from "svelte" - - export let chooseModal - export let save + export let onConfirm + export let onCancel export let showProgressCircle = false - let selectedScreens = [] - const blankScreen = "createFromScratch" - const dispatch = createEventDispatcher() - function setScreens() { - dispatch("save", { - screens: selectedScreens, - }) - } + let selectedScreens = [] + let templates = getTemplates($store, $tables.list) $: blankSelected = selectedScreens?.length === 1 $: autoSelected = selectedScreens?.length > 0 && !blankSelected - let templates = getTemplates($store, $tables.list) - - const confirm = async () => { - if (autoSelected) { - setScreens() - await save() - } else { - setScreens() - chooseModal(1) - } - } const toggleScreenSelection = table => { if (selectedScreens.find(s => s.table === table.name)) { selectedScreens = selectedScreens.filter( @@ -56,25 +36,25 @@ } } - onDestroy(() => { - selectedScreens = [] - }) + const confirmScreenSelection = async () => { + await onConfirm(selectedScreens) + }
confirm()} + onConfirm={confirmScreenSelection} + {onCancel} disabled={!selectedScreens.length} size="L" > - Please select the screens you would like to add to your application. - Autogenerated screens come with CRUD functionality. - + + Please select the screens you would like to add to your application. + Autogenerated screens come with CRUD functionality. + Blank screen
{ if (!event.detail.startsWith("/")) { - url = "/" + event.detail + screenUrl = "/" + event.detail } - url = sanitizeUrl(url) - - if (routeExists(url, roleId)) { + screenUrl = sanitizeUrl(screenUrl) + if (routeExists(screenUrl)) { routeError = "This URL is already taken for this access role" } else { - routeError = "" + routeError = null } } - const routeExists = (url, roleId) => { - return $allScreens.some( + const routeExists = url => { + const roleId = get(selectedAccessRole) || "BASIC" + return get(allScreens).some( screen => screen.routing.route.toLowerCase() === url.toLowerCase() && screen.routing.roleId === roleId ) } - onDestroy(() => { - screenName = "" - url = "" - }) + const confirmScreenDetails = async () => { + await onConfirm({ + screenName, + screenUrl, + }) + } chooseModal(0)} - onConfirm={() => save()} + onConfirm={confirmScreenDetails} + {onCancel} cancelText={"Back"} - disabled={!screenName || !url || routeError} + disabled={!screenName || !screenUrl || routeError} >
diff --git a/packages/builder/src/components/design/NavigationPanel/ScreenWizard.svelte b/packages/builder/src/components/design/NavigationPanel/ScreenWizard.svelte index 144592035c..fd37490c53 100644 --- a/packages/builder/src/components/design/NavigationPanel/ScreenWizard.svelte +++ b/packages/builder/src/components/design/NavigationPanel/ScreenWizard.svelte @@ -3,141 +3,137 @@ import NewScreenModal from "components/design/NavigationPanel/NewScreenModal.svelte" import sanitizeUrl from "builderStore/store/screenTemplates/utils/sanitizeUrl" import { Modal, notifications } from "@budibase/bbui" - import { store, selectedAccessRole, allScreens } from "builderStore" + import { store, selectedAccessRole } from "builderStore" import analytics, { Events } from "analytics" + import { get } from "svelte/store" - let newScreenModal - let navigationSelectionModal - let screenDetailsModal - let screenName = "" - let url = "" - let selectedScreens = [] + let pendingScreen let showProgressCircle = false - let routeError - let createdScreens = [] - $: roleId = $selectedAccessRole || "BASIC" + // Modal refs + let newScreenModal + let screenDetailsModal - const createScreens = async () => { - for (let screen of selectedScreens) { - let test = screen.create() - createdScreens.push(test) - analytics.captureEvent(Events.SCREEN.CREATED, { - template: screen.id || screen.name, - }) - } - } + // External handler to show the screen wizard + export const showModal = () => { + newScreenModal.show() - const save = async () => { - showProgressCircle = true - try { - await createScreens() - for (let screen of createdScreens) { - await saveScreens(screen) - } - await store.actions.routing.fetch() - selectedScreens = [] - createdScreens = [] - screenName = "" - url = "" - } catch (error) { - notifications.error("Error creating screens") - } + // Reset state when showing modal again + pendingScreen = null showProgressCircle = false } - const saveScreens = async draftScreen => { - let existingScreenCount = $store.screens.filter( - s => s.props._instanceName == draftScreen.props._instanceName - ).length - if (existingScreenCount > 0) { - let oldUrlArr = draftScreen.routing.route.split("/") - oldUrlArr[1] = `${oldUrlArr[1]}-${existingScreenCount + 1}` - draftScreen.routing.route = oldUrlArr.join("/") + // Creates an array of screens, checking and sanitising their URLs + const createScreens = async screens => { + if (!screens?.length) { + return } + showProgressCircle = true - let route = url ? sanitizeUrl(`${url}`) : draftScreen.routing.route - if (draftScreen) { - if (!route) { - routeError = "URL is required" - } else { - if (routeExists(route, roleId)) { - routeError = "This URL is already taken for this access role" - } else { - routeError = "" + try { + for (let screen of screens) { + // Check we aren't clashing with an existing URL + if (hasExistingUrl(screen.routing.route)) { + let suffix = 2 + let candidateUrl = makeCandidateUrl(screen, suffix) + while (hasExistingUrl(candidateUrl)) { + candidateUrl = makeCandidateUrl(screen, ++suffix) + } + screen.routing.route = candidateUrl } - } - if (routeError) return false + // Sanitise URL + screen.routing.route = sanitizeUrl(screen.routing.route) - if (screenName) { - draftScreen.props._instanceName = screenName - } + // Use the currently selected role + screen.routing.roleId = get(selectedAccessRole) || "BASIC" - draftScreen.routing.route = route - draftScreen.routing.roleId = roleId + // Create the screen + await store.actions.screens.save(screen) - await store.actions.screens.save(draftScreen) - if (draftScreen.props._instanceName.endsWith("List")) { - try { + // Analytics + if (screen.template) { + analytics.captureEvent(Events.SCREEN.CREATED, { + template: screen.template, + }) + } + + // Add link in layout for list screens + if (screen.props._instanceName.endsWith("List")) { await store.actions.components.links.save( - draftScreen.routing.route, - draftScreen.routing.route.split("/")[1] + screen.routing.route, + screen.routing.route.split("/")[1] ) - } catch (error) { - notifications.error("Error creating link to screen") } } + + // Refresh routing info since we have new screens + await store.actions.routing.fetch() + } catch (error) { + console.log(error) + notifications.error("Error creating screens") + } + + showProgressCircle = false + } + + // Checks if any screens exist in the store with the given route and + // currently selected role + const hasExistingUrl = url => { + const roleId = get(selectedAccessRole) || "BASIC" + const screens = get(store).screens.filter(s => s.routing.roleId === roleId) + return !!screens.find(s => s.routing?.route === url) + } + + // Constructs a candidate URL for a new screen, suffixing the base of the + // screen's URL with a given suffix. + // e.g. "/sales/:id" => "/sales-1/:id" + const makeCandidateUrl = (screen, suffix) => { + let url = screen.routing?.route || "" + if (url.startsWith("/")) { + url = url.slice(1) + } + if (!url.includes("/")) { + return `/${url}-${suffix}` + } else { + const split = url.split("/") + return `/${split[0]}-${suffix}/${split.join("/")}` } } - const routeExists = (route, roleId) => { - return $allScreens.some( - screen => - screen.routing.route.toLowerCase() === route.toLowerCase() && - screen.routing.roleId === roleId - ) - } - - export const showModal = () => { - newScreenModal.show() - } - - const setScreens = evt => { - selectedScreens = evt.detail.screens - } - - const chooseModal = index => { - /* - 0 = newScreenModal - 1 = screenDetailsModal - 2 = navigationSelectionModal - */ - if (index === 0) { - newScreenModal.show() - } else if (index === 1) { + // Handler for NewScreenModal + const confirmScreenSelection = async templates => { + // Handle template selection + if (templates?.length > 1) { + // Autoscreens, so create immediately + const screens = templates.map(template => template.create()) + await createScreens(screens) + } else { + // Empty screen, so proceed to the next modal + pendingScreen = templates[0].create() screenDetailsModal.show() - } else if (index === 2) { - navigationSelectionModal.show() } } + + // Handler for ScreenDetailsModal + const confirmScreenDetails = async ({ screenName, screenUrl }) => { + if (!pendingScreen) { + return + } + pendingScreen.props._instanceName = screenName + pendingScreen.routing.route = screenUrl + await createScreens([pendingScreen]) + } - + newScreenModal.show()} />