Repository navigation
Navigation bar customization - #2168
davidwaroquiers wants to merge 9 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2168 +/- ##
==========================================
+ Coverage 82.48% 82.56% +0.07%
==========================================
Files 91 91
Lines 8782 8815 +33
==========================================
+ Hits 7244 7278 +34
+ Misses 1538 1537 -1
🚀 New features to boost your workflow:
|
gpetretto
left a comment
There was a problem hiding this comment.
LGTM. I just left a couple of minor comments.
| If no entry is explicitly marked as the default, the first entry is used. | ||
| The built-in navigation marks Samples as the default to preserve the existing landing page. | ||
| The navigation must contain at least one entry, and duplicate view IDs, multiple defaults, and malformed entries are rejected during server configuration. | ||
| View IDs unknown to the installed web app are ignored by it. |
There was a problem hiding this comment.
Shouldn't it also give an error maybe. The most likely cause for seems a typo and it could be difficult to identify. Not sure how easy it is to implement.
There was a problem hiding this comment.
I would keep it like that for now, it's not so straightforward to do it at deployment time (at least for when there will be custom views).
| const navigation = resolveNavigation(serverInfo); | ||
| const defaultEntry = navigation.find((entry) => entry.default); | ||
|
|
||
| return defaultEntry?.routeName || navigation[0]?.routeName || NAVIGATION_VIEWS.samples.routeName; |
There was a problem hiding this comment.
In which case this would arrive to NAVIGATION_VIEWS.samples.routeName? And if there is such a case, what happens if the samples route is hidden from the top bar? Will the user end up there in any case?
There was a problem hiding this comment.
It's only if all the view ids are unknown to the frontend (or are considered unavailable for some reason - to be defined in the future), then it indeed goes to samples. Hiding a navigation tab does not remove the route, it remains accessible. So you would basically end up there if there is no "proper" navigation bar (which is different from having no defined navigation tab, these tabs could be "wrong" or "unavailable"). Unlikely case but at least there is a fallback.
|
This should be ready for review @ml-evs. My next step would be to introduce a "blocks table route" to be able to visualize blocks "independently" from their items (likely in read-only mode, to be discussed and possibly relaxed later). I would keep that "blocks table route" hidden by default in the navigation bar and if someone wants to show it he should use this new custom NAVIGATION config. Any comment or feedback is welcome of course (note that the above screenshots are available on our previews if useful). Note that I kept two comments from @gpetretto open in case you want to comment or have a request/solution for these. |
This PR implements the first phase of configurable navigation described in the corresponding issue: #2035
Deployments can now configure the visible views, their order, labels, and optional icons through NAVIGATION in config.json or PYDATALAB_NAVIGATION.
Existing deployments retain the current navigation by default.
The backend exposes this configuration through /info, and the frontend resolves stable view IDs through a central view catalog.
Future phases
A later phase will extend the same approach to support:
The same registry-based approach also provides a path towards a dedicated data blocks view, now that blocks are stored in their own collection (#2069 ).
Example of a customized nav bar with the following NAVIGATION in config.json: