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]

Reply via email to