Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Create screen" component context option #14255

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
componentStore,
selectedComponent,
componentTreeNodesStore,
screenStore,
} from "stores/builder"
import { findComponent, getChildIdsForComponent } from "helpers/components"
import { goto, isActive } from "@roxi/routify"
Expand Down Expand Up @@ -48,6 +49,14 @@
["Ctrl+Enter"]: () => {
$goto(`./:componentId/new`)
},
["CloneNodeToScreen"]: async component => {
const newScreen = await screenStore.createScreenFromComponent(component)
if (newScreen) {
notifications.success("Created screen successfully")
} else {
notifications.error("Error creating screen from selection.")
}
},
["Delete"]: component => {
// Don't show confirmation for the screen itself
if (component?._id === $selectedScreen.props._id) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ const getContextMenuItems = (component, componentCollapsed) => {
disabled: noPaste,
callback: () => keyboardEvent("v", true),
},
{
icon: "WebPage",
name: "Create screen",
visible: true,
disabled: false,
callback: () => keyboardEvent("CloneNodeToScreen", false),
},
{
icon: "Export",
name: "Eject block",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
builderStore,
} from "stores/builder"
import { auth } from "stores/portal"
import { get } from "svelte/store"
import getTemplates from "templates"
import { Roles } from "constants/backend"
import { capitalise } from "helpers"
Expand Down Expand Up @@ -53,11 +52,13 @@

for (let screen of screens) {
// Check we aren't clashing with an existing URL
if (hasExistingUrl(screen.routing.route)) {
if (
screenStore.hasExistingUrl(screen.routing.route, screenAccessRole)
) {
let suffix = 2
let candidateUrl = makeCandidateUrl(screen, suffix)
while (hasExistingUrl(candidateUrl)) {
candidateUrl = makeCandidateUrl(screen, ++suffix)
let candidateUrl = screenStore.makeCandidateUrl(screen, suffix)
while (screenStore.hasExistingUrl(candidateUrl, screenAccessRole)) {
candidateUrl = screenStore.makeCandidateUrl(screen, ++suffix)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than all this logic (some of which I know was pre-existing), we should use our getSequentialName utility which is designed to do exactly this!

}
screen.routing.route = candidateUrl
}
Expand Down Expand Up @@ -91,32 +92,6 @@
}
}

// Checks if any screens exist in the store with the given route and
// currently selected role
const hasExistingUrl = url => {
const roleId = screenAccessRole
const screens = get(screenStore).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.slice(1).join("/")}`
}
}

// Handler for NewScreenModal
export const show = newMode => {
mode = newMode
Expand Down
113 changes: 112 additions & 1 deletion packages/builder/src/stores/builder/screens.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,25 @@ import { derived, get } from "svelte/store"
import { cloneDeep } from "lodash/fp"
import { Helpers } from "@budibase/bbui"
import { RoleUtils, Utils } from "@budibase/frontend-core"
import { findAllMatchingComponents } from "helpers/components"
import {
findAllMatchingComponents,
makeComponentUnique,
} from "helpers/components"
import {
layoutStore,
appStore,
componentStore,
navigationStore,
selectedComponent,
componentTreeNodesStore,
} from "stores/builder"
import { createHistoryStore } from "stores/builder/history"
import { API } from "api"
import BudiStore from "../BudiStore"
import { Roles } from "constants/backend"
import { Screen } from "templates/Screen"
import sanitizeUrl from "helpers/sanitizeUrl"
import { capitalise } from "helpers"

export const INITIAL_SCREENS_STATE = {
screens: [],
Expand All @@ -36,6 +44,9 @@ export class ScreenStore extends BudiStore {
this.updateSetting = this.updateSetting.bind(this)
this.sequentialScreenPatch = this.sequentialScreenPatch.bind(this)
this.removeCustomLayout = this.removeCustomLayout.bind(this)
this.hasExistingUrl = this.hasExistingUrl.bind(this)
this.makeCandidateUrl = this.makeCandidateUrl.bind(this)
this.createScreenFromComponent = this.createScreenFromComponent.bind(this)

this.history = createHistoryStore({
getDoc: id => get(this.store).screens?.find(screen => screen._id === id),
Expand Down Expand Up @@ -176,6 +187,106 @@ export class ScreenStore extends BudiStore {
}
}

/**
* Checks if any screens exist in the store with the given route and
* currently selected role
*
* @param {string} screen
* @param {string} screenAccessRole
* @returns {boolean}
*/
hasExistingUrl(url, screenAccessRole) {
const state = get(this.store)
const roleId = screenAccessRole
const screens = state.screens.filter(s => s.routing.roleId === roleId)
return !!screens.find(s => s.routing?.route === url)
}

/**
* Constructs a candidate URL for a new screen, appending a given suffix
* to the base of the screen's URL
*
* @param {string} screen
* @param {number} suffix
* @example "/sales/:id" => "/sales-1/:id"
* @returns {string} The candidate url.
*/
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.slice(1).join("/")}`
}
}

/**
* Takes a component object and creates a brand new screen.
* The default screen path is '/new-screen' with a corresponding nav link.
* If a screen url collision is found, a numeric suffix will be added.
* @example "/new-screen-2"
*
* @param {object} component - The component to be copied
* @param {string} [accessRole='BASIC'] - Access role for the new screen. (default=BASIC)
*
* @returns {object|null} The new screen or null if it failed
*/
async createScreenFromComponent(component, accessRole = Roles.BASIC) {
const defaultNewScreenURL = sanitizeUrl(`/new-screen`)
let componentCopy = Helpers.cloneDeep(component)

const screen = new Screen()
.route(defaultNewScreenURL)
.instanceName(`New Screen`)

const screenJSON = screen.json()

if (this.hasExistingUrl(defaultNewScreenURL, accessRole)) {
let suffix = 2
let candidateUrl = this.makeCandidateUrl(screenJSON, suffix)
while (this.hasExistingUrl(candidateUrl, accessRole)) {
candidateUrl = this.makeCandidateUrl(screenJSON, ++suffix)
}
screenJSON.routing.route = candidateUrl
}

// Ensure the component _id values are made unique.
const uniqueComponentCopy = makeComponentUnique(componentCopy)

screenJSON.props._children.push(uniqueComponentCopy)
screenJSON.routing.roleId = accessRole

try {
// Create the screen
const newScreen = await this.save(screenJSON)

// Add a new nav link
await navigationStore.saveLink(
screenJSON.routing.route,
capitalise(screenJSON.routing.route.split("/")[1]),
accessRole
)

// Select the new node.
componentStore.update(state => {
state.selectedComponentId = uniqueComponentCopy._id
return state
})

// Ensure the node is expanded
componentTreeNodesStore.makeNodeVisible(uniqueComponentCopy._id)

return newScreen
} catch (e) {
console.error(e)
return null
}
}

/**
* Core save method. If creating a new screen, the store will sync the target
* screen id to ensure that it is selected in the builder
Expand Down
15 changes: 14 additions & 1 deletion packages/client/src/components/ClientApp.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,24 @@
// Provide contexts
setContext("sdk", SDK)
setContext("component", writable({ id: null, ancestors: [] }))
setContext("context", createContextStore())

const globalContext = createContextStore()
setContext("context", globalContext)

let dataLoaded = false
let permissionError = false
let embedNoScreens = false
let cachedActiveScreenId

// Clear component context entries from the global context
// when the active page changes.
const purgeGlobalComponentContext = screenId => {
if (!screenId || cachedActiveScreenId == screenId) return
cachedActiveScreenId = screenId
globalContext.actions.clearComponentContext()
}

$: purgeGlobalComponentContext($screenStore?.activeScreen?._id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the purpose of this, but I think we should come up with a cleaner way of handling it than trying to whitelist all properties that we want to persist between screen changes. I have a few ideas in mind... For now I think it would be better to take this out of this PR as it's a separate issue, and we can tackle it elsewhere.


// Determine if we should show devtools or not
$: showDevTools = $devToolsEnabled && !$routeStore.queryParams?.peek
Expand Down
33 changes: 33 additions & 0 deletions packages/client/src/stores/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,45 @@ export const createContextStore = parentContext => {
observers.forEach(cb => cb(key))
}

/**
* Purge the global context of any component entries
*
* rowSelection (Legacy) - used by the old Table component.
*/
const clearComponentContext = () => {
context.update(ctx => {
const {
device,
snippets,
user,
user_RefreshDatasource,
state,
query,
url,
rowSelection,
} = {
...ctx,
}
return {
device,
snippets,
user,
user_RefreshDatasource,
state,
query,
url,
rowSelection,
}
})
}

return {
subscribe: totalContext.subscribe,
actions: {
provideData,
provideAction,
observeChanges,
clearComponentContext,
},
}
}
Loading