Skip to content

Conversation

@evnchn
Copy link
Collaborator

@evnchn evnchn commented Nov 22, 2025

Motivation

So in ui.navigate.to we kinda "cheat" and call await client.sub_pages_router._handle_navigate(path) directly.

While it works to navigate the page, userspace handlers (ui.on('sub_pages_navigate', ...)) do not work, as seen in #5487

Implementation

Don't cheat I guess? Go through the event listener system.

  • We go through the event listener(s) and invoke them with mocked dictionaries.
  • Userspace handlers work, of course, as of this change.
  • While, client.sub_pages_router._handle_navigate is still being ran because of the following code.

class SubPagesRouter:
def __init__(self, request: Request | None) -> None:
on('sub_pages_open', lambda event: self._handle_open(event.args))
on('sub_pages_navigate', lambda event: self._handle_navigate(event.args))

Progress

  • I chose a meaningful title that completes the sentence: "If applied, this PR will..."
  • The implementation is complete.
  • Pytests have been added (c0547ef).
  • Documentation has been added (or is not necessary).

@evnchn evnchn added the bug Type/scope: Incorrect behavior in existing functionality label Nov 22, 2025
@evnchn evnchn linked an issue Nov 22, 2025 that may be closed by this pull request
3 tasks
@evnchn evnchn requested a review from rodja November 22, 2025 14:54
@falkoschindler
Copy link
Contributor

@evnchn I'm curious what @rodja thinks about this PR.
But either way, we should probably add a pytest that fails on main and passes with this PR.

@falkoschindler falkoschindler added the review Status: PR is open and needs review label Nov 22, 2025
@falkoschindler falkoschindler added this to the 3.5 milestone Nov 22, 2025
Copy link
Member

@rodja rodja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

@rodja
Copy link
Member

rodja commented Nov 22, 2025

run_javascript(f'emitEvent("sub_pages_navigate", "{path}");') might be simpler, but then we will need a roundtrip to the client.

@evnchn
Copy link
Collaborator Author

evnchn commented Nov 22, 2025

I did think of that (my initial instinct), but did not go with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Type/scope: Incorrect behavior in existing functionality review Status: PR is open and needs review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ui.navigate.to not triggering sub_pages_navigate event

3 participants