Refactor navigation bits

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

@@ -56,8 +56,9 @@ def validate_navigation(cls, navigation, values):
         if "pages" not in values:
             return navigation
 
-        if navigation is None:
-            return Navigation(pages=[page.id for page in values["pages"]])
+        # AM: test these cases well
+        navigation = navigation or Navigation()
+        navigation.pages = navigation.pages or [page.id for page in values["pages"]]
         return navigation
 
     def __init__(self, *args, **kwargs):

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

@@ -14,8 +14,9 @@ def _validate_pages(pages):
     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
+    # 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/nav_bar.py

@@ -1,62 +1,71 @@
 from __future__ import annotations
+from collections.abc import Mapping
 
 import itertools
-from typing import List, Literal, Optional, cast
+from typing import List, Optional, cast, Literal, Dict
 
 from dash import html
-from pydantic import validator
+from pydantic import validator, Field
 
 from vizro.models import VizroBaseModel
 from vizro.models._models_utils import _log_call
 from vizro.models._navigation._navigation_utils import _validate_pages
 from vizro.models._navigation.nav_item import NavItem
-from vizro.models.types import NavPagesType
 
 
 class NavBar(VizroBaseModel):
     """Navigation bar to be used as a selector for `Navigation`.
 
     Args:
         type (Literal["navbar"]): Defaults to `"navbar"`.
-        pages (Optional[NavPagesType]): See [`NavPagesType`][vizro.models.types.NavPagesType].
-            Defaults to `None`.
-        items (List[NavItem]): See [`NavItem`][vizro.models.NavItem]. Defaults to `[]`.
+        pages (Optional[Dict[str, List[str]]]): A dictionary with a page group title as key and a list of page IDs as
+            values.
+        items (Optional[List[NavItem]]): See [`NavItem`][vizro.models.NavItem]. Defaults to `[]`.
     """
 
-    type: Literal["navbar"] = "navbar"
-    pages: Optional[NavPagesType] = None
-    items: List[NavItem] = []
+    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, always=True)(_validate_pages)
 
+    @validator("pages", pre=True)
+    def coerce_pages_type(cls, pages):
+        if isinstance(pages, Mapping):
+            return pages
+        return {page: [page] for page in pages}
+
     @validator("items", always=True)
-    def validate_items(cls, items, values):
-        if not items:
-            if isinstance(values.get("pages"), list):
-                return [NavItem(pages=[page]) for page in values["pages"]]
-            if isinstance(values.get("pages"), dict):
-                return [NavItem(pages=value) for page, value in values["pages"].items()]
+    def set_items(cls, items, values):
+        # AM: Will this check work correctly when pages not set?
+        if "pages" not in values:
+            return values
+
+        items = items or [NavItem(text=group_title, pages=pages) for group_title, pages in values["pages"].items()]
+
+        # AM: test works if set some icons but not others
+        for position, item in enumerate(items):
+            # There are only 6 looks icons. If there are more than 6 items, the icons will repeat.
+            item.icon = item.icon or f"looks_{position % 6 + 1}"
 
         return items
 
     @_log_call
-    def build(self, active_page_id):
-        return html.Div(
-            children=[
-                html.Div(
-                    children=[item.build(active_page_id=active_page_id) for item in self.items],
-                    className="nav-bar",
-                    id="nav_bar_outer",
-                ),
-                self._nav_panel_build(active_page_id=active_page_id),
-            ]
-        )
-
-    def _nav_panel_build(self, active_page_id):
-        for item in self.items:
-            pages = list(itertools.chain(*item.pages.values())) if isinstance(item.pages, dict) else item.pages
-            if active_page_id in pages:
-                return cast(NavBar, item.selector).build(active_page_id=active_page_id)
-
-            return html.Div(hidden=True, id="nav_panel_outer")
+    def build(self, *, active_page_id=None):
+        # We always show all the navitem buttons, but only show the accordion for the active page. This works because
+        # item.build only returns the nav_panel_outer Div when the item is active.
+        # In future maybe we should do this by showing all navigation panels and then setting hidden=True for all but
+        # one using a clientside callback?
+        built_items = [item.build() for item in self.items]
+        buttons = [item[item.id] for item in built_items]
+        if "nav_panel_outer" in built_items:
+            nav_panel_outer = built_items["nav_panel_outer"]
+        else:
+            # Active page is not in navigation, so hide navigation panel.
+            nav_panel_outer = html.Div(hidden=True, id="nav_panel_outer")
+
+        return html.Div([html.Div(buttons, className="nav-bar", id="nav_bar_outer"), nav_panel_outer])

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

@@ -2,12 +2,12 @@
 
 import itertools
 import os
-from typing import Literal, Optional
+from typing import Optional, Literal, Dict, List, Union
 
 import dash
 import dash_bootstrap_components as dbc
 from dash import html
-from pydantic import Field, root_validator, validator
+from pydantic import Field, root_validator, validator, PrivateAttr
 
 from vizro.models import VizroBaseModel
 from vizro.models._models_utils import _log_call
@@ -22,84 +22,59 @@ class NavItem(VizroBaseModel):
     Args:
         type (Literal["navitem"]): Defaults to `"navitem"`.
         pages (NavPagesType): See [NavPagesType][vizro.models.types.NavPagesType].
-        icon (str): Name of the icon from the Google Material Icon library. Defaults to "dashboard". For more available
-            icons visit [Google Material Icon library](https://fonts.google.com/icons). To turn off icon provide `""`.
-        max_text_length (int): Character limit for `text` argument. If the text exceeds the `max_text_length`,
-            it is automatically truncated and the full text is visible in the tooltip on hover. Defaults to 8.
+        icon (str): Name of the icon from the Google Material Icon library. For more available
+            icons visit [Google Material Icon library](https://fonts.google.com/icons).
         text (Optional[str]): Text to be displayed below the icon. It automatically gets truncated to the
             `max_text_length`. Defaults to `None`.
-        tooltip (Optional[str]): Text to be displayed in the icon tooltip on hover. It is automatically populated if
-            the `text` exceeds the `max_text_length`. Defaults to `None`.
-        selector (Optional[Accordion]): See [`Accordion`][vizro.models.Accordion]. Defaults to `None`.
     """
 
-    type: Literal["navitem"] = "navitem"
-    pages: NavPagesType
-    icon: str = Field(
-        "dashboard",
-        description="URI (absolute) of the embeddable content or icon name from Google Material Icon library.",
-    )
-    max_text_length: int = Field(8, description="Character limit for `text` argument.")
-    text: Optional[str] = Field(None, description="Text to be displayed below the icon.")
-    tooltip: Optional[str] = Field(None, description="Text to be displayed in the icon tooltip on hover.")
-    selector: Optional[Accordion] = None
+    type: Literal["navitem"] = "navitem"  # AM: nav_item?
+    pages: Optional[NavPagesType] = []
+    text: str = Field(
+        ..., description="Text to be displayed below the icon."
+    )  # 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()
 
     # Re-used validators
     _validate_pages = validator("pages", allow_reuse=True, always=True)(_validate_pages)
 
-    @root_validator
-    def set_text_and_tooltip(cls, values):
-        if values["text"] and (len(values["text"]) > values["max_text_length"]):
-            if values["tooltip"] is None:
-                values["tooltip"] = values["text"]
-            values["text"] = values["text"][: values["max_text_length"]]
-        return values
+    @_log_call  # can't do this in validator since it's private?
+    def pre_build(self):
+        from vizro.models._navigation.accordion import Accordion
 
-    @validator("selector", pre=True, always=True)
-    def set_selector(cls, selector, values):
-        if selector is None:
-            return Accordion(pages=values.get("pages"))
-        return selector
+        self._selector = Accordion(pages=self.pages)  # type: ignore[arg-type]
 
     @_log_call
-    def build(self, active_page_id):
-        icon_first_page = (
-            list(itertools.chain(*self.pages.values()))[0] if isinstance(self.pages, dict) else self.pages[0]
-        )
-        text_div = html.Div(children=[self.text], className="icon-text") if self.text else html.Div(className="hidden")
-
-        return dbc.Button(
-            id=self.id,
-            children=[
-                html.Div(
-                    children=[
-                        self._create_icon_div(),
-                        text_div,
-                    ],
-                    className="nav-icon-text",
-                ),
-                self._create_icon_tooltip(),
+    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
+
+        try:
+            first_page = dash.page_registry[first_page_id]
+        except KeyError as exc:
+            raise KeyError(
+                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.Div(html.Span(self.icon, className="material-symbols-outlined"), className="nav-icon-text"),
+                dbc.Tooltip(html.P(self.text), target=self.id, placement="bottom", className="custom-tooltip"),
             ],
+            id=self.id,
             className="icon-button",
-            href=dash.page_registry[icon_first_page]["relative_path"],
-            active=icon_first_page == active_page_id,
+            href=first_page["relative_path"],
+            active=item_active,
         )
 
-    def _create_icon_tooltip(self):
-        if self.tooltip:
-            tooltip = dbc.Tooltip(
-                children=html.P(self.tooltip),
-                target=self.id,
-                placement="bottom",
-                className="custom-tooltip",
-            )
-            return tooltip
-
-    def _create_icon_div(self):
-        if not self.icon:
-            return html.Div(className="hidden", id="nav_bar_outer")
-
-        if os.path.isabs(self.icon):
-            return html.Img(src=self.icon, className="nav-icon")
+        # Only build the selector (id="nav_panel_outer") if the item is active.
+        if item_active:
+            return html.Div([button, item._selector.build(active_page_id=active_page_id)])
 
-        return html.Span(self.icon, className="material-symbols-outlined")
+        return button

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

@@ -20,34 +20,32 @@ class Navigation(VizroBaseModel):
         pages (Optional[NavPagesType]): See [`NavPagesType`][vizro.models.types.NavPagesType].
             Defaults to `None`.
         selector (Optional[NavSelectorType]): See [`NavSelectorType`][vizro.models.types.NavSelectorType].
-            Defaults to `None`.)
+            Defaults to `None`.
     """
 
-    pages: Optional[NavPagesType] = None
-    selector: Optional[NavSelectorType] = None
+    pages: Optional[NavPagesType] = None  # AM: yes but NavPagesType: note breaking change, maybe put whole type hint in
+    selector: Optional[NavSelectorType] = None  # AM: yes
 
     # validators
     _validate_pages = validator("pages", allow_reuse=True, pre=True, always=True)(_validate_pages)
 
     @validator("selector", always=True)
     def set_selector(cls, selector, values):
-        if selector is None:
-            return Accordion(pages=values.get("pages"))
+        # AM: Will this check work correctly when pages not set?
+        if "pages" not in values:
+            return values
 
-        if isinstance(selector, NavBar):
-            if selector.pages is None and selector.items is None:
-                selector.pages = values.get("pages")
-                return selector
+        selector = selector or Accordion()
+        selector.pages = selector.pages or values["pages"]
         return selector
 
     @_log_call
     def build(self, *, active_page_id=None):
-        if isinstance(self.selector, NavBar):
-            return self.selector.build(active_page_id=active_page_id)
-        if isinstance(self.selector, Accordion):
-            return html.Div(
-                children=[
-                    html.Div(className="hidden", id="nav_bar_outer"),
-                    self.selector.build(active_page_id=active_page_id),
-                ]
-            )
+        selector = self.selector.build(active_page_id=active_page_id)
+        if "nav_bar_outer" not in selector:
+            # e.g. selector is Accordion and selector.build returns single html.Div with id="nav_panel_outer". This will
+            # make it match the case e.g. selector is NavBar and selector.build returns html.Div containing children
+            # with id="nav_bar_outer" and id="nav_panel_outer"
+            selector = html.Div([html.Div(className="hidden", id="nav_bar_outer"), selector])
+
+        return selector

vizro-core/tests/unit/vizro/models/test_dashboard.py

@@ -23,7 +23,6 @@ def dashboard_container():
             html.Div(id=f"vizro_version_{vizro.__version__}"),
             ActionLoop._create_app_callbacks(),
             dash.page_container,
-            html.Link(href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined", rel="stylesheet"),
         ],
         className="vizro_dark",
         fluid=True,