Add address filtering to RouterLogModal#9386
Conversation
Adds address log filtering modes the router log modal, with a button that cycles through them. The four modes you can cycle through are: * `none`: Do not filter by address * `with <address>`: Show traffic to and from <address> (specifically your own address, for now) * `from <address>`: Show traffic from <address> * `to <address>`: Show traffic to <address> Right now, the address given is always your own node's current address; therefore, this option is only available when you are connected to a router and have an address assigned. Otherwise the address filter mode will always be `none`. The default mode is `none` unless the user changes it, to be as consistent as possible with the current behavior.
|
Why did we go with a cycle rather than a dropdown? |
| * @type {boolean} | ||
| */ | ||
|
|
||
| /** |
There was a problem hiding this comment.
are the options for this clearly enumerated anywhere? It looks like they are independently implemented in a couple of different places
There was a problem hiding this comment.
This is just an effort to document the locals that this template expects; they're the variables passed in from NetSimRouterLogModal::render(). I feel like ejs has always had this problem of things being loosely coupled, and I don't know if there's a better approach.
|
Because I am lazy 😛 and I think if we do that for the address filtering then we should do it for the router filtering too, just to be consistent. Shall I switch to that now? |
| } else { | ||
| markup += i18n.logBrowserHeader_toggleAll() | ||
| } | ||
| markup += '</button>'; |
There was a problem hiding this comment.
Do we get es6 in ejs files? If so, this would be a great spot for string interpolation!
let headerText = locals.isAllRouterLogMode
? i18n.logBrowserHeader_toggleMine()
: i18n.logBrowserHeader.toggleAll();
let markup = `<button type="button" id="routerlog-toggle" class="pull-right btn btn-primary btn-mini">${ headerText }</button>`There was a problem hiding this comment.
I'm not sure, actually. I'll give it a try.
|
Haha, fair enough. Cycles of longer than two always just feel cumbersome; how hard would it be to switch to a dropdown? How weird would it be to provide consistency by also making address filtering a dropdown? |
| if (locals.isAllRouterLogMode) { | ||
| header = i18n.logBrowserHeader_all(); | ||
| } else { | ||
| header = i18n.logBrowserHeader_mine(); |
There was a problem hiding this comment.
any reason this isn't a ternary assignment?
|
Given that the content of the filter string has a defined syntax and serves to both indicate which mode we are in and curry data for the filter operation itself, it feels sufficiently complex to be pulled out into some kind of helper class. Thoughts? |
|
I can see how a helper class would be useful here; to be honest I'm hoping to convert this thing to React after this change, which may make said cleanup much easier. How do you feel about punting on the address filter type object until then? |
|
I feel very positively about that entire plan! |
|
I've converted the address filter and router filter controls to dropdowns; the PR description has been updated with new screenshots. PTAL. |
| * @property {string} currentTrafficFilter | ||
| * @property {string} localAddress | ||
| * @property {string} sortBy | ||
| * @property {boolean} sortDescending |
|
LGTM! |
Adds address log filtering modes the router log modal.
Motivation
The lesson about routing asks students to open up the router log browser and trace the path of their packets across the network. Right now this is very difficult because the router log browser live-updates with all traffic in the network, has limited scrollback, and can't be filtered by sender or recipient; in a full classroom you lose log entries as soon as you find them.
By adding these filters we allow students to view a subset of traffic relevant to them, while still seeing all routers; this gives them more opportunity to find relevant packets and their paths, without unrelated traffic interfering.
Behavior
The four modes you can select are:
none: Do not filter by addresswith <address>: Show traffic to and from (specifically your own address, for now)from <address>: Show traffic fromto <address>: Show traffic toThe dialog title updates to reflect the current filter mode.
Right now, the address given is always your own node's current address; therefore, this option is only available when you are connected to a router and have an address assigned. Otherwise the address filter mode will always be
none. The default mode isnoneunless the user changes it, to be as consistent as possible with the current behavior.The filter selection control is a dropdown, and we've converted the router filtering control to a dropdown too for consistency.
Screenshots
Trying all four modes (local router only):
Example of filtering to own traffic on all routers, to make it easier to trace packets relevant to you: