Skip to content
This repository was archived by the owner on Aug 13, 2018. It is now read-only.

Add ability to filter by socket id #34

Merged
merged 15 commits into from
Dec 3, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions chrome/locale/en-US/websocket-monitor.properties
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ pluralRule=1

websocketmonitor.summary.frameCount=%1$S frame;%1$S frames

# LOCALIZATION NOTE (websocketmonitor.ConnectionFilter.NoFilter): A label displayed
# to the user inside the connection ID dropdown filter. It shows that the user has
# not yet selected a connection to filter on, and should inform the user of the
# purpose of the dropdown.
websocketmonitor.ConnectionFilter.NoFilter=Filter by Socket ID

# LOCALIZATION NOTE (websocketmonitor.label.sent, websocketmonitor.label.received):
# A label for frames displayed in the frame list.
websocketmonitor.label.sent=Sent
Expand Down
84 changes: 84 additions & 0 deletions data/components/connection-filter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/* See license.txt for terms of usage */

define(function(require, exports/*, module*/) {

"use strict";

// Dependencies
const React = require("react");

// WebSockets Monitor
const { filterFrames } = require("../actions/frames");

// Shortcuts
const { span, select, option } = React.DOM;

/**
* This component renders a select element when unique
* webSocket connections > 1. Using this dropdown, the
* user can select the connection ID they are interested
* in seeing, and a reducer should filter out all other
* connections.
*/
var ConnectionFilter = React.createClass({
/** @lends ConnectionFilter */

displayName: "ConnectionFilter",

getInitialState() {
return {
uniqueConnections: []
};
},

handleChange(e) {
const { value } = e.target;
const currentFilter = this.props.frames.filter;

// Dispatch new filter, merging with old filter to
// retain text filter alongside this filter.
this.props.dispatch(filterFrames(
Object.assign({}, currentFilter, {
webSocketSerialID: value !== null ? Number(value) : null
})
));
},

// When the platform API supports it, this should be replaced
// with some API call listing only the current connections on
// the page.
componentWillReceiveProps({ frames }) {
frames.frames.forEach(frame => {
const { uniqueConnections } = this.state;

if (!uniqueConnections.includes(frame.webSocketSerialID)) {
this.setState({
uniqueConnections: [...uniqueConnections, frame.webSocketSerialID]
});
}
})
},

render() {
const { uniqueConnections } = this.state;
return (
uniqueConnections.length > 1 ?
select({
className: "ConnectionFilter",
value: this.props.frames.filter.webSocketSerialID,
onChange: this.handleChange
}, option({ value: null }, Locale.$STR("websocketmonitor.ConnectionFilter.NoFilter")),
uniqueConnections.map((id, i) => {
return option({key: i, value: id}, id);
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Not sure if the key attribute is useful here (and standard for the option element)?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is because we are mapping over uniqueConnections and generating children. it is necessary to avoid React complain about Warning: Each child in an array or iterator should have a unique "key" prop :)

See https://facebook.github.io/react/docs/multiple-components.html#dynamic-children

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see. Thanks for pointing me out this good practice :).

Florent

})
) :
// else, no-op
span()
);
}
});

// Exports from this module
exports.ConnectionFilter = ConnectionFilter;
});

17 changes: 5 additions & 12 deletions data/components/main-toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ const { Toolbar, ToolbarButton } = createFactories(require("reps/toolbar"));

// WebSockets Monitor
const { clear } = require("../actions/frames");
const { SearchBox } = require("./search-box");
const { SearchBox } = createFactories(require("./search-box"));
const { ConnectionFilter } = createFactories(require("./connection-filter"));

/**
* @template This object is responsible for rendering the toolbar
Expand All @@ -31,16 +32,6 @@ var MainToolbar = React.createClass({
}
},

componentDidMount: function() {
var toolbar = ReactDOM.findDOMNode(this.refs.toolbar);
SearchBox.create(toolbar);
},

componentWillUnmount: function() {
var toolbar = ReactDOM.findDOMNode(this.refs.toolbar);
SearchBox.destroy(toolbar);
},

// Commands

onTogglePause: function() {
Expand Down Expand Up @@ -89,7 +80,9 @@ var MainToolbar = React.createClass({
),
ToolbarButton({bsSize: "xsmall", onClick: this.onSwitchPerspective},
perspectiveLabel
)
),
SearchBox(this.props),
ConnectionFilter(this.props)
Copy link
Member

Choose a reason for hiding this comment

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

My guess is that we may rather put the connection filter on the left of the SearchBox. Otherwise, the appearance of the dropdown list moves the search box and may disturb the user.

Also I am a bit curious why the SearchBox is in componentDidMount. Ping @janodvarko?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I agree. It would be better if the dropdown appeared to the left of the search box.

I assume SearchBox is in componentDidMount because it is a legacy non-react component, and needs to be injected directly into the DOM through a constructor method.

Because they (SearchBox and ConnectionFilter) are both float: right and css is weird, I need the ability to position SearchBox above ConnectionFilter in the markup. This would mean being able to put SearchBox in the render method, and thus rewriting SearchBox to react. I'll have a stab at it and see what I can do.

)
);
},
Expand Down
99 changes: 46 additions & 53 deletions data/components/search-box.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,77 +4,70 @@ define(function(require, exports/*, module*/) {

"use strict";

// Dependencies
const React = require("react");

// WebSockets Monitor
const { filterFrames } = require("../actions/frames");

// Shortcuts
const { input } = React.DOM;

/**
* TODO docs xxxHonza: use ReactJS.
* This component renders a search box that allows the
* user to filter on the content of the frames in the
* list. It dispatches a "filterFrames" event with the
* updated text filter.
*/
var SearchBox =
var SearchBox = React.createClass({
/** @lends SearchBox */
{
create: function(parentNode) {
var doc = parentNode.ownerDocument;
var win = doc.defaultView;
var toolbar = doc.querySelector(".mainPanel .toolbar");

// Search box
var searchBox = doc.createElement("input");
searchBox.setAttribute("class", "devtools-searchinput");
searchBox.setAttribute("type", "search");
searchBox.setAttribute("results", "true");
toolbar.appendChild(searchBox);

searchBox.addEventListener("command", this.onChange.bind(this, searchBox), false);
searchBox.addEventListener("input", this.onChange.bind(this, searchBox), false);
searchBox.addEventListener("keypress", this.onKeyPress.bind(this, searchBox), false);

this.handleThemeChange = this.handleThemeChange.bind(this, searchBox);
win.addEventListener("theme-changed", this.handleThemeChange);

displayName: "SearchBox",

getInitialState() {
return {
text: ""
};
},

componentDidMount() {
document.defaultView.addEventListener("theme-changed", this.handleThemeChange);
},

destroy: function(parentNode) {
var doc = parentNode.ownerDocument;
var searchBox = doc.querySelector(".searchBox");
searchBox.remove();
win.removeEventListener("theme-changed", this.handleThemeChange);
componentWillUnmount() {
document.defaultView.removeEventListener("theme-changed", this.handleThemeChange);
},

handleThemeChange: function(searchBox, event) {
var data = event.data;
var win = searchBox.ownerDocument.defaultView;
handleThemeChange(event) {
const data = event.data;

// Reset the filter if Firebug theme has been activated or deactivated.
if (data.newTheme == "firebug" || data.oldTheme == "firebug") {
searchBox.value = "";
this.dispatch(win, {
text: ""
});
this.onChange("");
}
},

onKeyPress: function(searchBox/*, event*/) {
this.onSearch(searchBox);
},

onChange: function(searchBox/*, event*/) {
this.onSearch(searchBox);
},
onChange(text) {
const currentFilter = this.props.frames.filter;

onSearch: function(searchBox) {
var win = searchBox.ownerDocument.defaultView;
this.dispatch(win, {
text: searchBox.value
});
// Dispatch new filter, merging with old filter to
// retain connectionId filter alongside this filter.
this.props.dispatch(filterFrames(
Object.assign({}, currentFilter, { text })
));
this.setState({ text });
},

dispatch: function(win, filter) {
var event = new win.MessageEvent("firebug.sdk/chrome-event", {
data: {
method: "onSearch",
args: filter
}
render() {
return input({
className: "devtools-searchinput",
type: "search",
results: "true",
value: this.state.text,
onChange: e => this.onChange(e.target.value)
});
win.dispatchEvent(event);
}
};
});

// Exports from this module
exports.SearchBox = SearchBox;
Expand Down
26 changes: 26 additions & 0 deletions data/css/connection-filter.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* See license.txt for terms of usage */

/******************************************************************************/
/* Connection Filter Dropdown */

.theme-dark .ConnectionFilter {
color: var(--theme-content-color1);
border: none;
background: rgba(170, 170, 170, .3);
}

.theme-firebug .ConnectionFilter {
color: #333;
border-radius: 2px;
}

.ConnectionFilter {
float: right;
height: 22px;
border: 1px solid rgb(204, 204, 204);

background: rgb(255, 255, 255) none repeat scroll 0% 0%;
box-shadow: 0px 1px 1px rgba(0, 0, 0, 0.075) inset;
color: rgb(85, 85, 85);
}

61 changes: 43 additions & 18 deletions data/reducers/frames.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,51 +95,76 @@ function addFrames(state, newFrames) {
});

// Apply filter on incoming frames.
if (newState.filter.text) {
return filterFrames(newState, newState.filter);
}

return newState;
return filterFrames(newState, newState.filter);
Copy link
Member Author

Choose a reason for hiding this comment

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

This fixed an issue of new frames not showing up when switching back to "No socket ID filter". I was not able to see any adverse effects of doing this, so I left it in.

}

function filterFrames(state, filter) {
var frames;

var summary = {
totalSize: 0,
startTime: 0,
endTime: 0,
frameCount: 0
};
var { frames } = state;
var summary = null;

if (filter.text) {
frames = state.frames.filter(frame => {
summary = {
totalSize: 0,
startTime: 0,
endTime: 0,
frameCount: 0
};

frames = frames.filter(frame => {
var data = frame.data;
if (data.payload && data.payload.indexOf(filter.text) != -1) {

// Exclude where data is null (events). Events have no payload
if (data && data.payload && data.payload.indexOf(filter.text) != -1) {
summary.totalSize += data.payload.length;
summary.startTime = summary.startTime ? summary.startTime : data.timeStamp;
summary.endTime = data.timeStamp;
summary.frameCount++;
return true;
}
});
} else {
summary = null;
}

if (filter.webSocketSerialID) {
summary = {
totalSize: 0,
startTime: 0,
endTime: 0,
frameCount: 0
};

frames = frames.filter(frame => {
var data = frame.data;
if (frame.webSocketSerialID === filter.webSocketSerialID) {

// If data is null, this is not an actual frame, but an event
// like "connect" or "disconnect". We still want to keep it
// in the list, though.
if (data) {
summary.totalSize += data.payload.length;
summary.startTime = summary.startTime ? summary.startTime : data.timeStamp;
summary.endTime = data.timeStamp;
summary.frameCount++;
}
return true;
}
});
}

return Object.assign({}, state, {
filter: {
text: filter.text,
webSocketSerialID: filter.webSocketSerialID,
frames: frames,
summary: summary,
}
});
}

function clear(state) {
// All data are cleared except of the current filter.
// All data is cleared except for the current filters.
var newState = getInitialState();
newState.filter.text = state.filter.text;
newState.filter.webSocketSerialID = state.filter.webSocketSerialID;
return newState;
}

Expand Down
1 change: 1 addition & 0 deletions data/view.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<link href="./css/styles.css" rel="stylesheet">
<link href="./css/frame-table.css" rel="stylesheet">
<link href="./css/frame-list.css" rel="stylesheet">
<link href="./css/connection-filter.css" rel="stylesheet">
<link href="./css/search-box.css" rel="stylesheet">
<link href="./css/no-service-warning.css" rel="stylesheet">
<script src="../node_modules/keymaster/keymaster.js"></script>
Expand Down