msyavuz commented on issue #45248:
URL: https://github.com/apache/superset/issues/45248#issuecomment-6343372348
Thanks for the thorough read, @michael-s-molina . I've folded most of this
into the body; summary of what changed:
**Taken as-is**
- Motivation reworded with your paragraphs. Chat already renders from
`App.tsx`, so "only SQL Lab" was wrong.
- Route renamed to `/extensions/pages/:pageId` as a method on
`ExtensionsView`, plain segment (ids are dotted). Note this goes the other way
from where #44101 landed after the
`<path:view_id>` review; the PR should follow the SIP once we settle.
- `list` and `pages` reserved as publisher names at manifest validation.
`PUBLISHER_PATTERN` accepts both today.
- Always render the shell on an unknown id; client shows not-found. Open
question 5 removed.
- `href` deferred. Open question 4 removed.
- `MenuItem` is now a discriminated union and `registerMenuItem` throws on
an invalid item; `registerView` throws on a duplicate id (it's a `Map`, so it
silently overwrites today).
- `Page` union change called out in migration notes. Numbering fixed.
- Added a "Possible follow-up" section: a backend page declaration keyed
on the same `global.page` view id fits without changing the frontend-only path.
**Reusing the `Extensions` permission**
`Extensions` is in `ADMIN_ONLY_VIEW_MENUS`, so only Admin has `can_read`
on it by default. Gating the shell route with it would make every extension
page admin-only. Login-only stays for
the shell; I recorded this under rejected alternatives. Per-page
permissions belong to the backend-declaration follow-up.
**`page` vs `core.navigation.openView`**
I agree a page is a view at `global.page` and that "open this view" is a
command. Where I land differently is on the navbar item: the host needs a
static target to render a real link
(middle-click, open in new tab, a11y), highlight the active tab, and skip
an unregistered page. With command + args the host would special-case that one
command id and read its args to
get those back, and `MenuItem` has no args field today. So the body now
does both: `page` stays declarative, and its click handler is a public built-in
`core.navigation.openView(pageId)`,
so keybindings and other locations can open a page without a navbar item.
Does that cover what you were after, or do you still see a reason to drop
`page`?
--
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]
