amaannawab923 commented on code in PR #45522:
URL: https://github.com/apache/superset/pull/45522#discussion_r4401489152


##########
superset-frontend/src/features/home/Menu.tsx:
##########
@@ -199,6 +200,108 @@ const StyledImage = styled(Image)`
 
 const { useBreakpoint } = Grid;
 
+/**
+ * What the two helpers below need of a contributed page, and no more, so the
+ * menu does not take a dependency on the pages registry. One shape shared by
+ * both, so a field one of them reads cannot go missing from the other.
+ */
+type ContributedPage = {
+  name: string;
+  path: string;
+  nav?: { label?: string; category?: string };
+  permitted?: boolean;
+};
+
+/**
+ * Folds pages contributed by extensions into the server-rendered menu.
+ *
+ * The navbar is built on the server from FAB's menu, which an extension
+ * cannot write to: an entry there needs a `menu_access` permission created
+ * at boot, and extensions register in the browser. A page that opted in with
+ * `nav` is therefore added here, client-side, so that a contributed page has
+ * somewhere a user could find it at all.
+ *
+ * A page without `nav.category` sits at the top level. One that names a
+ * category goes into that group of the Settings menu instead, which is
+ * where an administrative screen belongs -- see `withExtensionSettings`.
+ */
+export function withExtensionPages(
+  menu: MenuObjectProps[],
+  pages: ContributedPage[],
+): MenuObjectProps[] {
+  // A page the user may not open is left out rather than shown and refused:
+  // the server answers 403 for it, so an entry here would only advertise a
+  // dead end. The page stays registered, so a direct link still explains
+  // itself rather than rendering nothing.
+  const contributed = pages.filter(
+    page => page.nav && !page.nav.category && page.permitted !== false,
+  );
+  if (!contributed.length) {
+    return menu;
+  }
+
+  return [

Review Comment:
   Capped in 8b41b5f, with the tail behind one dropdown -- the option that 
stays inside the extensions path rather than changing host chrome.
   
   Three contributed entries stay on the bar; beyond that, two stay and the 
rest fold into "More", the dropdown counting against the cap. So the bar holds 
the same number of contributed slots whether one extension is installed or 
thirty. Nothing is dropped: every page is either on the bar or in the dropdown.
   
   The fold reuses what the menu already does rather than adding anything. An 
item with children and no `url` renders as a dropdown, and `isFrontendRoute` is 
computed per render for children as well as items, so a folded page is still an 
SPA link rather than a full reload. Its key is unchanged too, so a page inside 
the dropdown still highlights while it is open -- tested, along with the case 
where an unpermitted page is excluded from the fold as well as from the bar.
   
   Being straight about what this does and does not do: it bounds growth, it 
does not promise the bar never wraps. No single number can. Three contributed 
entries still wrap at 1280px, and your own four host items fill the row at 
820px with no extensions installed at all. What it removes is the unbounded 
case -- ten entries running past the viewport and painting over the Settings 
menu.
   
   The complete answer is making the bar responsive, or giving antd's overflow 
back by dropping `disabledOverflow`. I did not do that here: it puts the host's 
own entries under responsive measurement for the first time, nothing in the 
suite asserts anything about that prop today, and `StyledMainNav` styles 
`.ant-menu-submenu-horizontal` for a different thing than the rest item would 
be. That feels like yours to drive, in its own PR with a visual check -- and if 
you would rather have it that way than this cap, I am happy to drop the cap 
when it lands.
   



-- 
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