michael-s-molina commented on PR #43559:
URL: https://github.com/apache/superset/pull/43559#issuecomment-5438722271
We're using VS Code's extension model as our north star for this
contribution system, so it's worth naming a concept it has that we're missing
here: `registerLeftBarView(view, trigger, panel)` bundles two things VS Code
keeps separate — a **view container** (a rail slot: icon, id, order, no content
of its own) and a **view** (content registered *into* a container). Because the
two are fused 1:1 here, this needs its own bespoke registration function, its
own registry (`leftBarViews.ts`), and its own host (`LeftBarViewHost.tsx`) —
none of which any future rail-style location (right sidebar, elsewhere) could
reuse without inventing yet another `registerXView` function.
The actual precedent is VS Code's `viewsContainers` + `views`: a container
is a rail slot; views register into it by referencing its id, and multiple
views can stack inside one container. Mapped onto our code-registration model
(no manifest), that's two generic primitives instead of a per-module function:
```ts
// new — generic, not sqlLab-specific
interface ViewContainer {
id: string;
name: string;
icon: ComponentType; // the rail trigger
description?: string;
order?: number;
}
registerViewContainer(location: string, container: ViewContainer):
Disposable;
getViewContainers(location: string): ViewContainer[];
// unchanged — views.registerView(view, location, component) already exists;
// it just needs to accept a registered container's id as `location`, not
// only the static SqlLabLocation values it validates against today.
```
Concretely for this PR:
- `LeftBarView`/`registerLeftBarView`/`leftBarViews.ts` go away.
`sqlLab.leftSidebar` becomes just a location that hosts containers.
- The trigger becomes a container's `icon`; the panel becomes a normal view
registered into that container via the existing `views.registerView`.
- Explorer becomes a container core registers at startup through the same
API, instead of the `TAB_EXPLORER_ID`/`builtinIcon()` special-casing in
`LeftBarRail.tsx` and `useLeftBarTabs.ts`.
- `LeftBarRail` becomes a generic rail host (read
`getViewContainers(location)` for the icon strip, `getViews(activeContainerId)`
for content) — reusable by any future rail location, not sqlLab-specific.
One real gap to close, not just a rename: `location` today is a closed,
static taxonomy (`SqlLabLocation`) that `registerView` never validates because
every value is guaranteed to have a host reading it. Once `location` can also
be a container id minted at runtime, `registerView` needs to check the target
against a live registry (static locations ∪ registered container ids) and
reject an unknown one — warn + inert `Disposable`, the same pattern
`registerLeftBarView` already uses for a duplicate id — instead of silently
orphaning the view. `registerViewContainer` should likewise refuse an id
colliding with a reserved static location name.
--
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]