Conversation
|
@codex any thoughts? |
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
https://github.com/iamgp/dash_dashkit/blob/f5115d1c199d180eced0cd6b8c798f5166a5067c/src/dashkit/breadcrumbs.py#L36-L35
[P2] Make breadcrumbs callback resilient to missing url component
The callback subscribes to Input("url", "pathname") without allow_optional=True. create_layout exposes include_theme_manager=False, which removes the dcc.Location with id url; in that configuration Dash raises at startup because the callback references a component that does not exist. _update_page_config already marks the same input optional for this reason. Consider adding allow_optional=True or documenting that a dcc.Location(id="url") is mandatory.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| label = ( | ||
| page.get("name") or page.get("title") | ||
| if page |
There was a problem hiding this comment.
[P1] Guard breadcrumb label when parent path not registered
The label calculation page.get("name") or page.get("title") if page else … evaluates page.get(...) before the if page branch, so when the current URL includes a segment that has no entry in page_registry (e.g., only /reports/detail is registered), page is None and the callback raises AttributeError: 'NoneType' object has no attribute 'get'. As soon as a nested route lacks a dedicated parent page, the breadcrumb callback crashes and nothing renders. The expression should check that page exists before calling .get or use parentheses to avoid the short‑circuit ordering.
Useful? React with 👍 / 👎.
Summary
Breadcrumbscomponent that builds navigation trail from Dashpage_registryBreadcrumbsfrom package and document in changelogTesting
uv run --group dev task checkhttps://chatgpt.com/codex/tasks/task_e_68b44407fbe48321ab5cdbfa41d77e62