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

[feat] bind event handler to env and window #869

Open
shouldsee opened this issue Sep 28, 2022 · 2 comments
Open

[feat] bind event handler to env and window #869

shouldsee opened this issue Sep 28, 2022 · 2 comments

Comments

@shouldsee
Copy link
Contributor

shouldsee commented Sep 28, 2022

Is your feature request related to a problem? Please describe.
The supposed problem would be, if I have windows with the same name in different env, then accroding to event_handler, their events would cross-talk.

Describe the solution you'd like
change the event_handler to binds to a precise (env,win) pair.

Describe alternatives you've considered
second thought. As long as the "target" emited from frontend interaction is paired with backend callback, it should be controllable. Currently "target" points to the interacting window, thus currently there isn't a way of controlling the message target when creating windows and plots.

Additional context

current code looks like this. event message example.

                event = {'eid': 'text_callbacks',
                 'event_type': 'KeyPress',
                 'key': 'd',
                 'key_code': 68,
                 'pane_data': {'command': 'window',
                               'content': 'This is a write demo notepad. Type below. Delete '
                                          'clears text:<br>a',
                               'contentID': '43af9d32-5185-4167-8ef2-52b34b3c84e2',
                               'height': None,
                               'i': 1,
                               'id': 'window_3b1fede0134b74',
                               'inflate': True,
                               'title': '',
                               'type': 'text',
                               'width': None},
                 'target': 'window_3b1fede0134b74'}

callback registration

    def register_event_handler(self, handler, target):
        assert callable(handler), 'Event handler must be a function'
        assert self.use_socket, 'Must be using the incoming socket to '\
            'register events to web actions'
        if target not in self.event_handlers:
            self.event_handlers[target] = []
        self.event_handlers[target].append(handler)

callback execution

        def on_message(message):
            message = json.loads(message)
            if 'command' in message:
                # Handle server commands
                if message['command'] == 'alive':
                    if 'data' in message and message['data'] == 'vis_alive':
                        logger.info('Visdom successfully connected to server')
                        self.socket_alive = True
                        self.socket_connection_achieved = True
                    else:
                        logger.warn('Visdom server failed handshake, may not '
                                    'be properly connected')
            if 'target' in message:
                for handler in list(
                        self.event_handlers.get(message['target'], [])):
                    handler(message)
@shouldsee shouldsee changed the title [feat] event handler is shared across envs [feat] bind event handler to env and window Sep 28, 2022
@JackUrb
Copy link
Contributor

JackUrb commented Oct 17, 2022

This is an interesting discussion, as the current callback functionality is definitely limited. I think your second thought direction is likely more compelling, as this opens up Visdom towards a model of having remote procedure calls. Still, it's also more complicated as choosing what actions to register for frontend windows is somewhat unclear.

Ultimately I'd be happy to accept either solution though.

@shouldsee
Copy link
Contributor Author

This is an interesting discussion, as the current callback functionality is definitely limited. I think your second thought direction is likely more compelling, as this opens up Visdom towards a model of having remote procedure calls. Still, it's also more complicated as choosing what actions to register for frontend windows is somewhat unclear.

Ultimately I'd be happy to accept either solution though.

Hi jack thanks for reading. I am currently using the second-thought solution, using "target" as pointer to backend-registered callback, which is working fine so far, with the frontend message sent through the following line of js-hacker.

app._reactRootContainer._internalRoot.current.child.stateNode.sendSocketMessage(msg);

I am currently heavily using vis.text to create custom html window with js-hacker callback func and it's working like a charm so far. Definitely worth adding rpc design into visdom since it's definitely a highly frequent demand!

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

No branches or pull requests

2 participants