Tidy comments and make sure nav_control_panel hidden when not wanted

vizro-core/src/vizro/models/_dashboard.py

@@ -123,8 +123,11 @@ def _make_page_layout(self, page: Page):
         # Arrangement
         header = html.Div(children=[page_title, theme_switch], className="header", id="header_outer")
         nav_control_elements = [dashboard_title, nav_panel, control_panel]
-        # TODO: check behaviour when all of nav_control_elements are hidden.
-        nav_control_panel = html.Div(nav_control_elements, className="nav_control_panel")
+        nav_control_panel = (
+            html.Div(nav_control_elements, className="nav_control_panel")
+            if any(not getattr(element, "hidden", False) for element in nav_control_elements)
+            else html.Div(hidden=True)
+        )
 
         left_side = html.Div(children=[nav_bar, nav_control_panel], className="left_side", id="left_side_outer")
         right_side = html.Div(children=[header, component_container], className="right_side", id="right_side_outer")

vizro-core/src/vizro/models/_navigation/_navigation_utils.py

@@ -3,8 +3,6 @@
 from vizro.managers import model_manager
 
 
-# AM: handle different dict/list types better - build in _pages property that is list? Or provide argumenyt to
-# validator?
 def _validate_pages(pages):
     """Reusable validator to check if provided Page IDs exist as registered pages."""
     from vizro.models import Page
@@ -15,11 +13,6 @@ def _validate_pages(pages):
     # page[0] gives the page model ID.
     registered_pages = [page[0] for page in model_manager._items_with_type(Page)]
 
-    # required to auto-populate the navigation.selector.pages with registered pages
-    # if pages is None:
-    #     return registered_pages
-    # AM: shouldn't be needed here? Only want to autopopulate at top-level.
-    #
     if not pages_as_list:
         raise ValueError("Ensure this value has at least 1 item.")
 

vizro-core/src/vizro/models/_navigation/accordion.py

@@ -29,7 +29,6 @@ class Accordion(VizroBaseModel):
 
     _validate_pages = validator("pages", allow_reuse=True)(_validate_pages)
 
-    # AM: test Navigation(pages=[1, 2], selector=Accordion() works
     @validator("pages", pre=True)
     def coerce_pages_type(cls, pages):
         if isinstance(pages, Mapping):

vizro-core/src/vizro/models/_navigation/nav_bar.py

@@ -24,14 +24,13 @@ class NavBar(VizroBaseModel):
     """
 
     type: Literal["navbar"] = "navbar"  # AM: nav_bar?
-    # pages: Optional[NavPagesType] = None
     pages: Optional[Dict[str, List[str]]] = Field(
         {}, description="A dictionary with a page group title as key and a list of page IDs as values."
     )
     items: Optional[List[NavItem]] = []  # AM: think about name
 
     # validators
-    _validate_pages = validator("pages", allow_reuse=True, pre=True)(_validate_pages)
+    _validate_pages = validator("pages", allow_reuse=True)(_validate_pages)
 
     @validator("pages", pre=True)
     def coerce_pages_type(cls, pages):
@@ -43,7 +42,6 @@ def coerce_pages_type(cls, pages):
     def pre_build(self):
         self.items = self.items or [NavItem(text=group_title, pages=pages) for group_title, pages in self.pages.items()]
 
-        # AM: test works if set some icons but not others
         for position, item in enumerate(self.items, 1):
             item.icon = item.icon or f"filter_{position}" if position <= 9 else "filter_9+"
 

vizro-core/src/vizro/models/_navigation/nav_item.py

@@ -32,7 +32,7 @@ class NavItem(VizroBaseModel):
     pages: Optional[NavPagesType] = []
     text: str = Field(
         ..., description="Text to be displayed below the icon."
-    )  # Maybe call label. This just does tooltip for now.
+    )  # AM: Maybe call label. This just does tooltip for now.
     icon: Optional[str] = Field(None, description="Icon name from Google Material Icon library.")
 
     _selector: Accordion = PrivateAttr()
@@ -49,7 +49,6 @@ def pre_build(self):
     @_log_call
     def build(self, *, active_page_id=None):
         # _selector is an Accordion, so _selector._pages is guaranteed to be Dict[str, List[str]].
-        # AM: refactor to make private property for this in Accordion etc.
         all_page_ids = list(itertools.chain(*self._selector.pages.values()))
         first_page_id = all_page_ids[0]
         item_active = active_page_id in all_page_ids
@@ -61,11 +60,12 @@ def build(self, *, active_page_id=None):
                 f"Page with ID {first_page_id} cannot be found. Please add the page to `Dashboard.pages`"
             ) from exc
 
-        # remove nesting nav-icon-text now no text?
         button = dbc.Button(
             [
                 html.Span(self.icon, className="material-symbols-outlined"),
-                dbc.Tooltip(html.P(self.text), target=self.id, placement="bottom", className="custom-tooltip"),
+                # TODO: commented out until we insert styling for the tooltip or find a better way to display it (e.g.
+                # try Dash mantine components tooltip?).
+                # dbc.Tooltip(html.P(self.text), target=self.id, placement="bottom", className="custom-tooltip"),
             ],
             id=self.id,
             className="icon-button",

vizro-core/src/vizro/models/_navigation/navigation.py

@@ -24,10 +24,10 @@ class Navigation(VizroBaseModel):
     """
 
     pages: Optional[NavPagesType] = None  # AM: yes but NavPagesType: note breaking change, maybe put whole type hint in
-    selector: Optional[NavSelectorType] = None  # AM: yes
+    selector: Optional[NavSelectorType] = None
 
     # validators
-    _validate_pages = validator("pages", allow_reuse=True, pre=True)(_validate_pages)
+    _validate_pages = validator("pages", allow_reuse=True)(_validate_pages)
 
     @_log_call
     def pre_build(self):