sadpandajoe commented on code in PR #44101: URL: https://github.com/apache/superset/pull/44101#discussion_r4333393699
########## docs/developer_docs/extensions/extension-points/settings.md: ########## @@ -0,0 +1,114 @@ +--- +title: Settings Menu +sidebar_position: 4 +--- + +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Settings Menu Extension Points + +The **Settings** dropdown in the top navigation bar can list entries contributed by extensions, each opening a full-page view the extension renders. This is the extension point for things like an extension's own admin or configuration screen. + +Unlike the SQL Lab extension points, these locations are not declared in `extension.json`: the `contributions.views` and `contributions.menus` manifest fields only accept SQL Lab scopes. Extensions register against them in code instead, from the extension's entry point or `activate` function. Review Comment: An extension that registers its view, command, and menu item inside an exported `activate` function gets nothing in the Settings menu: `ExtensionsLoader.loadModule()` only runs the module factory (`factory()`) and never calls `activate`. Should this say registration must happen at module load, or that the entry point has to call the function itself? ########## docs/developer_docs/extensions/extension-points/settings.md: ########## @@ -0,0 +1,114 @@ +--- +title: Settings Menu +sidebar_position: 4 +--- + +<!-- +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +--> + +# Settings Menu Extension Points + +The **Settings** dropdown in the top navigation bar can list entries contributed by extensions, each opening a full-page view the extension renders. This is the extension point for things like an extension's own admin or configuration screen. + +Unlike the SQL Lab extension points, these locations are not declared in `extension.json`: the `contributions.views` and `contributions.menus` manifest fields only accept SQL Lab scopes. Extensions register against them in code instead, from the extension's entry point or `activate` function. + +| Extension Point | ID | Views | Menus | Description | +| ------------------ | ---------------------- | ----- | ----- | -------------------------------------------------------- | +| **Settings menu** | `global.settingsMenu` | — | ✓ | Entries in the Settings dropdown (`primary` group only) | +| **Settings panel** | `global.settingsPanel` | ✓ | — | Full-page views opened from those entries | + +## How the pieces fit together + +1. Register a view at `global.settingsPanel`. The host renders it at `/extensions/view/<view id>`, with the view's `name` as the page heading. +2. Register a command whose callback navigates to that route. +3. Register a menu item at `global.settingsMenu` in the `primary` group, pointing at that command. The host lists it under an **Extensions** group in the Settings dropdown, using the command's `title` as the label and its `description` as the tooltip. + +Only the `primary` group is rendered; items registered as `secondary` or `context` never appear in the Settings dropdown. Clicking an entry runs its command, and if the command throws or rejects, the host shows the error as a toast. + +The view page is available to every signed-in user while extensions are enabled, and the Settings entry appears for every user the extension loads for. The view's own backend calls enforce whatever permissions it needs. + +## View ids in the URL + +A view id may contain `/`. The route matches any number of path segments, so `/extensions/view/my-ext/settings` and `/extensions/view/my-ext%2Fsettings` both resolve `my-ext/settings`. Passing the id through `encodeURIComponent` is safe, including ids that contain `%`. + +While the extension providing a view is still loading, the page shows a loading indicator; once extensions have finished loading, an id that nothing registered shows a "could not be loaded" placeholder instead. + +## Toasts + +The `window` namespace shows transient toasts to the current user. It is not tied to any view, so it works from a command callback as well as from a rendered component: + +```typescript +import { window as supersetWindow } from '@apache-superset/core'; + +supersetWindow.showInformationMessage('Settings saved'); +supersetWindow.showWarningMessage('Sync is running behind', { duration: 6000 }); +supersetWindow.showErrorMessage('Could not reach the service'); +``` + +Each method takes the message and an optional `{ duration }` in milliseconds; when omitted, the host's default for that severity applies. Importing the namespace under an alias keeps it from shadowing the browser's global `window`. + +## Example + +This example adds a "My Extension Settings" entry to the Settings dropdown that opens the extension's own settings page: + +```typescript +import { + commands, + menus, + views, + window as supersetWindow, +} from '@apache-superset/core'; +import SettingsPanel from './SettingsPanel'; + +const VIEW_ID = 'my-extension.settings'; + +views.registerView( + { id: VIEW_ID, name: 'My Extension Settings' }, + 'global.settingsPanel', + SettingsPanel, +); + +commands.registerCommand( + { + id: 'my-extension.openSettings', + title: 'My Extension Settings', + description: 'Configure My Extension', + }, + () => { + globalThis.location.assign( Review Comment: On a deployment served under a subdirectory (`SUPERSET_APP_ROOT`), `globalThis.location.assign('/extensions/view/...')` skips the app root and lands on a 404, since a native navigation doesn't get the router's basename. Could the example build the URL with the app-root helper, or note that it assumes a root deployment? ########## superset-frontend/src/pages/ExtensionView/index.tsx: ########## @@ -0,0 +1,113 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useEffect, useState } from 'react'; +import { useParams } from 'react-router-dom'; +import { t } from '@apache-superset/core/translation'; +import { Loading } from '@superset-ui/core/components'; +import SubMenu, { SubMenuProps } from 'src/features/home/SubMenu'; +import ExtensionsLoader from 'src/extensions/ExtensionsLoader'; +import { useRegisteredView, useResolveView } from 'src/core/views'; + +/** + * Generic full-page host for a single extension-registered view, reached at + * `/extensions/view/:viewId+`. Extensions cannot render their own views + * directly (`resolveView`/`useResolveView` are host-internal, not part of + * the public `@apache-superset/core` SDK) -- they register a view at + * `GlobalLocations.settings.panel` and a matching command at + * `GlobalLocations.settings.menu` whose callback navigates here, and the + * host resolves and renders it. + * + * Uses the reactive `useResolveView`, not the plain `resolveView`: this + * page is reachable by a direct/full navigation (a bookmark, a page + * refresh, or an extension's own menu command falling back to + * `window.location.assign` in the absence of an SDK-level SPA-navigation + * primitive), and the providing extension's own code loads asynchronously + * -- a non-reactive resolve would permanently commit to the "could not be + * loaded" placeholder from before that load finishes. Until the extension + * loader settles, an unregistered id shows a loading state rather than + * that placeholder, so a slow load and a wrong id look different. + */ + +/** + * The router's `history` (v4) runs `decodeURI` on the pathname before + * `useParams` sees it. That pass decodes everything except the URI's + * reserved characters, so an id navigated to via `encodeURIComponent` + * arrives with `%2F` (and friends) still escaped but every other sequence, + * including a literal `%` sent as `%25`, already decoded. A second full + * `decodeURIComponent` would throw on an id that legitimately contains + * `%`, so only the reserved-character escapes that first pass leaves + * behind are decoded here. + */ +const RESERVED_CHARACTER_ESCAPES = /%(2F|3A|40|26|3D|2B|24|2C|3B|3F|23)/gi; + +const decodeViewId = (viewId: string | undefined): string => + (viewId ?? '').replace(RESERVED_CHARACTER_ESCAPES, escaped => Review Comment: A view id that itself contains an escape-shaped sequence resolves to a different id. For a view registered as `my-ext/literal%2Fname`, the documented `encodeURIComponent` navigation produces `%252F`; history decodes that to `%2F`, and this replace then turns it into `/`, so the host looks up `my-ext/literal/name` and shows the "could not be loaded" placeholder (or another registered view). The docs say ids containing `%` are safe, and the percent test only covers a trailing `%`. Could the id be decoded exactly once from the raw location, and a test added with an id like `my-ext/literal%2Fname`? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
