amaannawab923 commented on code in PR #45522:
URL: https://github.com/apache/superset/pull/45522#discussion_r4400663254
##########
superset-frontend/src/features/home/Menu.tsx:
##########
@@ -456,7 +562,7 @@ export function Menu({
className="main-nav"
selectedKeys={activeTabs}
disabledOverflow
- items={menu.map(item => {
+ items={withExtensionPages(menu, contributedPages).map(item => {
Review Comment:
Checked this one and I do not think it holds, though the reading of line 619
is right.
`RightMenu` does get the raw `menu`, but the drawer never enumerates it:
`mobileMenuItems` is built from scratch and its only use of the prop is
`menu?.find(item => item.name === "Dashboards")`. So decorating it would change
nothing visible -- and it would bust the `useMemo`, since `withExtensionPages`
returns a fresh array whenever any page is contributed, recomputing the drawer
on every render.
The host is in the same position. Rendering the drawer with a contributed
top-level page, a contributed `Manage` page, and host `Charts` and `Annotation
Layers` all present, the drawer contains only Dashboards, About and the user
section -- host `Charts` and host `Annotation Layers` are absent for exactly
the same reason. The drawer is not an incomplete copy of the nav; it is an
index of the `mobileSupported` route set, and every route it links to carries
that flag. A contributed page has no such flag, and `MobileRouteGuard` renders
`MobileUnsupported` for it, so linking it would be a link to an error screen.
Worth adding that `MOBILE_CONSUMPTION_MODE` is off by default, so a narrow
viewport normally gets the `inline` navbar with contributed entries in it.
Making contributed pages work there is a real feature -- a manifest field
for mobile capability, `MobileRouteGuard` consulting the registry rather than a
per-route flag (the pages share one catch-all, so the per-page decision cannot
be expressed in the route table as it stands), and the drawer appending from a
filtered list. That seems like its own change rather than part of this one.
--
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]
