feat: DH-21898: Add self-contained routing for widget#1342
feat: DH-21898: Add self-contained routing for widget#1342jnumainville wants to merge 1 commit intodeephaven:mainfrom
Conversation
|
|
||
| ## Overview | ||
|
|
||
| This plan covers local routing features for deephaven.ui: path-based navigation within a widget's `/local/` route space, declarative routing, and route parameter extraction. |
There was a problem hiding this comment.
I want to mention that I'm not tied to local, I'm sure there is something better. This is just the one I've been working with for prototyping.
Some other options:
~: short, unlikely for a user to want it, and has some precedence as user space, but ugly
_ or __: similar, some precedence but ugly
at is an interesting one, implies scope but a bit awkward
sub for subpath
|
|
||
| ### Path Resolution | ||
|
|
||
| The path is derived from the `/local/` segment in the URL. Widgets are served at routes like `<base_url>/embed/widget/<pq_name>/<widget_name>/local/…` — the `/local/` prefix separates platform routing from user routing. `use_path()` returns only the portion **after** `/local/`, which is the widget's own route space. `/local/` should not be used anywhere else in the URL to avoid conflicts. |
There was a problem hiding this comment.
I'm not really a fan of the local name. I'm assuming we want to add a specific path specifier here in case we someday want to add another path to a widget URL ourselves... Maybe just ui? Or - even? Not sure. local makes the URL much longer which isn't great. I kinda like - because it's pretty clear it's something special/going to the next segment.
E.g. /iriside/embed/widget/q/w/-/dashboard/settings
@dsmmcken for any comments.
There was a problem hiding this comment.
- is fine with me, as is ui
How does this impact not /embed paths, like if I am viewing it in dashboard?
How will this work in consoles?
There was a problem hiding this comment.
Yeah I think we want the path delimiter for future proofing especially.
The /embed path is just an example here. Any paths, including code studios and dashboards function the same in that they have whatever their base path is, then the delimiter, then the subpath. So for dashboards for example <base_url>/dashboard/<dashboardID>/<delimiter>/….
If we don't think - is ugly in the path I would prefer that just because the shorter the better. Maybe it is good if it's noticeable even.
|
|
||
| Before implementing absolute cross-widget routing, several prerequisites should be addressed, some of which were discovered while experimenting with the `WidgetPath`/`EnterpriseWidgetPath` resolution: | ||
|
|
||
| 1. **Improved URL routes**: Enterprise should support cleaner paths like `/dashboard/<query_name>/<widget>`, and community should use a path param instead of a query param for the widget name. |
There was a problem hiding this comment.
We'll need TODOs for these, and a deprecation of the query parameter used.
| ### API | ||
|
|
||
| ```python | ||
| def use_navigate() -> Callable[..., None]: |
There was a problem hiding this comment.
If we're adding absolute on use_path, I think it makes sense to add that here as well. Then you could devise a link to an entirely different URL.
That all being said... do we implement the relative path as the only way to start? Or should absolute be there right from the start?
There was a problem hiding this comment.
I specifically excluded the absolute navigation. It's tied up with the WidgetPath/EnterpriseWidgetPath problems I specified, especially the history integration. Right now it seems like I have to replace the window.location instead of the window.history, which is not the experience I'd want to provide.
I added absolute to use_path as informational until then, but it could be excluded if we think it would be confusing.
Add self-contained routing for a widget.
Future work for absolute navigation (between different widgets) is laid out, but there are issues blocking it from being ready to implement without further work.