Skip to content

Add socket_id to UIEventArguments; use new client.target where possible #4335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

falkoschindler
Copy link
Contributor

@falkoschindler falkoschindler commented Feb 10, 2025

This PR addresses issue #4016 and feature request #4218:

  • UI events should provide a socket ID to be used with the client.individual_target context.
  • ui.navigate, ui.download() and ui.notify() should adhere to the client.individual_target context.
  • ui.download() on the auto-index page (without using client.individual_target) should raise an exception.

Unfortunately, the current implementation has one main drawback:

  • The new socket ID argument affects all existing event argument classes. Therefore this change might break existing user code.

The PR is still a draft because we're thinking about alternative solutions, like automatically calling event handlers in the context of the current socket. This would automatically trigger notifications, downloads and navigations on the socket that caused the event. But it could also affect run_method() and run_javascript() calls, which might not be desired.

@falkoschindler falkoschindler added the feature Type/scope: New feature or enhancement label Feb 10, 2025
@falkoschindler
Copy link
Contributor Author

In f7d3fb7 I added a default '' to the socket_id field and removed it from the initializer. This way existing user code doesn't break when initializing event arguments without socket ID, and Python 3.8 and 3.9 don't complain about the order of positional and keyword arguments in derived classed. Given the limitations of dataclasses in Python <=3.9, I'm rather pleased with this solution.

@evnchn
Copy link
Collaborator

evnchn commented Apr 12, 2025

TL-DR: My solution is similar to yours, but I combine the client.individual_target with the client.id so that old code isn't affected.


I have thought about this for quite some time.

But first. A story to set the scene. Trust me it'll be relevant.

We have 4 "Anson" in the major. Therefore, it would not generally possible to say "hey Anson turn off the lights". As a discriminator, we would say "Big Anson" to refer to the oldest one, "Small Anson" to refer to the youngest one, and two other I don't really use since there are not in my year cohort.


@falkoschindler your solution hinges on the fact that now, we'll address everyone by their Socket ID (as assigned by Python-SocketIO) and the client_id (as assigned by NiceGUI. As such, everything which used to only have client_id needs to have an additional parameter. This explains the massive changes in all the code, and the backwards compatibility issues.


I propose that, instead of that, we use the name-discriminator approach, similar to "Big Anson" amongst the 4 Ansons.

The full client_id will therefore be deadbeef-face-4fed-abad-cafeaddeddad#1234, where 1234 is the client_discriminator (think: Discord in the old days).

Normal elements will be unaware of the change, since they get the plain client_id without the #x part. Even for auto-index page where it's all broadcasted.

Saying "Anson, stand up" is fine in a classroom, because you are fine with multiple people standing up.

But for ui.download and run_javascript where one-to-one response is expected, then we begin to care about the # of the client, and only handle the message if the # match.

We need explicitly "Big Anson, turn off the lights" since there can't be more than one person operating the light switch.


This means, instead of affecting every single element, only those who care about which client is sourced from, need to bear the responsibility of extracting the client_discriminator from most probably GenericEventArguments (say from a touch event) and shove that back into such sensitive functions which need it (ui.download and run_javascript)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Type/scope: New feature or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants