Skip to content
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

Allow sending arbitrary notifications to a Component. #35

Merged

Conversation

edelvalle
Copy link
Owner

@edelvalle edelvalle commented Jan 20, 2022

Adds the reactor.component.broadcast(channel, **kwargs) to send a notification to any Component subscribed to channel. he message is delivered in Component.notification(channel, **kwargs).

Adds the `reactor.component.broadcast(channel, **kwargs)` to send a
notification to any `Component` subscribed to `channel`. The message is
delivered in `Component.notification(channel, **kwargs)`.
Copy link
Collaborator

@jbjuin jbjuin left a comment

Choose a reason for hiding this comment

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

Hi @edelvalle
this looks good to me, I just tried it and it works.

I have a question : why did you add the _ on methods or arguments that are part of the "public" API ? They are intended to be used by the developers. My understanding is that in python those should be without the leading _, keeping it for internal functionality that should not be used by developers. For instance: _template_name or _subscriptions.

I also have another use case that do not work with this version: I want to pass a callable to a sub-component so that it can be called and the value is sent to the parent component. I will post an issue for this one. On my version it works flawlessly (reactor 2 + jinja template). I use this for an autocomplete component for instance...

@edelvalle
Copy link
Owner Author

I have a question : why did you add the _ on methods or arguments that are part of the "public" API ? They are intended to be used by the developers. My understanding is that in python those should be without the leading _, keeping it for internal functionality that should not be used by developers. For instance: _template_name or _subscriptions.

You are right, it is like kind of private because:

  • I don't want it to be exposed on the template context
  • I don't want the front-end to be able to call it
  • I want to signal in that way that those attributes are reserved for the component configuration.

I also have another use case that do not work with this version: I want to pass a callable to a sub-component so that it can be called and the value is sent to the parent component. I will post an issue for this one. On my version it works flawlessly (reactor 2 + jinja template). I use this for an autocomplete component for instance...

I see, in this case as I serialize the object state to the front-end, and serializing functions could be complicated. I would use this broadcast to talk to the parent component or any other component. The parent component could subscribe to a channel, where the child publishes updates.

  • The good: You can send updates of any kind from any component to any component, even on different tabs.
  • The bad: If you want to couple two components parent/child using this mechanism, you would have to generate a random ID for the channel of communication and pass it to the child. Why? Because otherwise all the parents in all tabs listen to the signal and get updated. Do I explain myself right?

So, should we merge this?

@jbjuin
Copy link
Collaborator

jbjuin commented Jan 24, 2022

I have a question : why did you add the _ on methods or arguments that are part of the "public" API ? They are intended to be used by the developers. My understanding is that in python those should be without the leading _, keeping it for internal functionality that should not be used by developers. For instance: _template_name or _subscriptions.

You are right, it is like kind of private because:

* I don't want it to be exposed on the template context

* I don't want the front-end to be able to call it

* I want to signal in that way that those attributes are reserved for the component configuration.

I see. Ok makes sense. Let's continue like this.

I also have another use case that do not work with this version: I want to pass a callable to a sub-component so that it can be called and the value is sent to the parent component. I will post an issue for this one. On my version it works flawlessly (reactor 2 + jinja template). I use this for an autocomplete component for instance...

I see, in this case as I serialize the object state to the front-end, and serializing functions could be complicated. I would use this broadcast to talk to the parent component or any other component. The parent component could subscribe to a channel, where the child publishes updates.

* The good: You can send updates of any kind from any component to any component, even on different tabs.

* The bad: If you want to couple two components parent/child using this mechanism, you would have to generate a random ID for the channel of communication and pass it to the child. Why? Because otherwise all the parents in all tabs listen to the signal and get updated. Do I explain myself right?

That's indeed another solution for this problem.

So, should we merge this?

Sure !

@edelvalle
Copy link
Owner Author

Ok, merging then. Now I will the the recv_ merge request.

@edelvalle edelvalle merged commit c388e09 into master Jan 25, 2022
@edelvalle edelvalle deleted the allow-sending-arbitrary-notifications-to-components branch January 25, 2022 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants