Pause and unpause about websocket-monitor HOT 16 CLOSED

@janodvarko What do you think?

Note that I took the liberty to change some of your code. Please tell me if these are fine.

Florent

from websocket-monitor.

Looks great!

The only thing I have is that new frames should not be collected when the monitor is paused. We might want to collect some summary info like number of missed frames total payload, etc. but not the frames.

The summary info should be displayed in the UI as well so, it's clear where exactly the pause happened (as an extra visually different table row, or a bubble in case of the list view).

It should work the same as RDPi.

Honza

from websocket-monitor.

Thanks for the feedback.

The only thing I have is that new frames should not be collected when the monitor is paused.

Right. May I ask why? As a user, I would personally rather expect the pause button to stop updating the list in the view until I unpause.

Florent

from websocket-monitor.

I am mainly worried about memory consumption and performance. The main first use case is that the user can use the button to stop updating/scrolling the content. And when the monitor is paused, the expectation is that it doesn't consume memory and doesn't slow down the browser (and listening to the frame service have an impact on performance). So, the second use case is that it is actually possible to 'pause' the entire feature if necessary.
I rather think that we should remove the listeners from the service entirely and give up collecting any info during pause...

Honza

from websocket-monitor.

I see. Let me try something then.

Florent

from websocket-monitor.

@fflorent:

Here is an info about content -> chrome message channel.

1. Use PanelView.postChromeMessage to send a message from the content to the chrome.
   Instance of PanelView is in data/main.js -> WebSocketsView

var data = {}; // must be JSON serializable
WebSocketsView.postChromeMessage("togglePause", data);

2. Handle in the panel, WsmPanel.onContentMessage()
   See the default implementation in PanelBase object (panel-base.js in FBSDK), it calls a method on the panel instance with the same name.

So, you just need to implement togglePause in WsmPanel

togglePause: function(data) {
}

Honza

from websocket-monitor.

Thanks Honza,

The main difficulty now is to communicate between the Application and the main class of the view in order to notify the user action.

I think of using the event lib in order to do that. What do you think?

Florent

from websocket-monitor.

No no, you should do it through actions (Redux) or callbacks passed through component properties (ReactJS)

What exactly do you want to do?

Honza

from websocket-monitor.

@fflorent Do you need any more help for this?

Note that WebSockets Monitor will be part of upcoming November DevEdition campaign and we should finish this issue it this week. Is that feasible?

Honza

from websocket-monitor.

Is that feasible?

That should be.

Florent

from websocket-monitor.

I managed to use a property as a callback. Does it sound better now?

Florent

from websocket-monitor.

I managed to use a property as a callback. Does it sound better now?

Yes, I also put some comments in the revision.
Honza

from websocket-monitor.

@janodvarko is it fine for you?

Florent

from websocket-monitor.

@fflorent Looks good!

The platform API landed and they are included in Firefox 44 now. The extension is already working with the Nightly build and so, try builds are not necessary. It should be a lot easier to test it now.

DevEdition 44 is not yet out, but will be soon and I'd like to release WebSocket Monitor today, so it's ready for broader audience of web developer (using the DevEdition).

Can you please merge into the master ASAP, or should I do it?
(there will be some conflicts since I had to adopt last minute platform changes and it's in the master now).

Btw. there are also some improvements that I think we should do as a follow ups. It's related to the fact that pause/unpause is async and:

1. The action doesn't have to make it to the backend (over RDP), we need error handling.
2. It might take some time to do the 'pause' on the backend and the UI should reflect that
3. We should make sure that the 'state' on the client is always in sync with the 'state' on the backend (e.g. we might disable the button while the action is in progress).

I'll file all those issues soon.

Honza

from websocket-monitor.

Alright, thanks for the feedback.

Can you please merge into the master ASAP, or should I do it?

Please do. I don't have time to do it myself.

Florent

from websocket-monitor.

Done

All seem to work nice, I'll release a new version today, it would be great if you can double check that the feature works as expected.

Honza

from websocket-monitor.