Commit 173b1a40 authored by James Long's avatar James Long Committed by Commit Bot

[Lorenz] Resolve TODOs

This change resolves the TODO(yjlong) artifacts scattered around the
codebase.

Bug: 1119942
Change-Id: I678317d11d8c7b60356d4dabf12d6b9d71055571
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2367647Reviewed-by: default avatarHenrique Nakashima <hnakashima@chromium.org>
Commit-Queue: James Long <yjlong@google.com>
Cr-Commit-Position: refs/heads/master@{#800590}
parent 8b51c94c
...@@ -93,8 +93,6 @@ class NodeFilterData { ...@@ -93,8 +93,6 @@ class NodeFilterData {
const deleteIndex = this.filterList.findIndex( const deleteIndex = this.filterList.findIndex(
filterEntry => filterEntry.name === nodeName); filterEntry => filterEntry.name === nodeName);
if (deleteIndex >= 0) { if (deleteIndex >= 0) {
// TODO(yjlong): If order turns out to be unimportant, just swap the
// last element and the deleted element, then pop.
this.filterList.splice(deleteIndex, 1); this.filterList.splice(deleteIndex, 1);
} }
} }
......
...@@ -8,6 +8,9 @@ import {GraphEdgeColor} from './display_settings_data.js'; ...@@ -8,6 +8,9 @@ import {GraphEdgeColor} from './display_settings_data.js';
import * as d3 from 'd3'; import * as d3 from 'd3';
// The radius of each node in the visualization.
const NODE_RADIUS = 5;
// Category10 colors pulled from https://observablehq.com/@d3/color-schemes. // Category10 colors pulled from https://observablehq.com/@d3/color-schemes.
const HULL_COLORS = ['#1f77b4', '#ff7f0e', '#2ca02c', '#d62728', '#9467bd', const HULL_COLORS = ['#1f77b4', '#ff7f0e', '#2ca02c', '#d62728', '#9467bd',
'#8c564b', '#e377c2', '#7f7f7f', '#bcbd22', '#17becf']; '#8c564b', '#e377c2', '#7f7f7f', '#bcbd22', '#17becf'];
...@@ -114,8 +117,7 @@ function addArrowMarkerDef(defs, id, color, length, width) { ...@@ -114,8 +117,7 @@ function addArrowMarkerDef(defs, id, color, length, width) {
defs.append('marker') defs.append('marker')
.attr('id', id) // 'graph-arrowhead-*' .attr('id', id) // 'graph-arrowhead-*'
.attr('viewBox', `0 -${halfWidth} ${length} ${width}`) .attr('viewBox', `0 -${halfWidth} ${length} ${width}`)
// TODO(yjlong): 5 is the hardcoded radius, change for dynamic radius. .attr('refX', length + NODE_RADIUS)
.attr('refX', length + 5)
.attr('refY', 0) .attr('refY', 0)
.attr('orient', 'auto') .attr('orient', 'auto')
.attr('markerWidth', length) .attr('markerWidth', length)
...@@ -799,8 +801,6 @@ class GraphView { ...@@ -799,8 +801,6 @@ class GraphView {
.attr('id', edge => edge.id) .attr('id', edge => edge.id)
.attr('gradientUnits', 'userSpaceOnUse')); .attr('gradientUnits', 'userSpaceOnUse'));
// TODO(yjlong): Determine if we ever want to render self-loops (will need
// to be a loop instead of a straight line) and handle accordingly.
this.edgeGroup_.selectAll('path') this.edgeGroup_.selectAll('path')
.data(inputEdges, edge => edge.id) .data(inputEdges, edge => edge.id)
.join(enter => enter.append('path')); .join(enter => enter.append('path'));
...@@ -847,8 +847,6 @@ class GraphView { ...@@ -847,8 +847,6 @@ class GraphView {
node.fx = node.x; node.fx = node.x;
node.fy = node.y; node.fy = node.y;
} }
// TODO(yjlong): Change this so the style is tied to whether the
// fx/fy are non-null instead of toggling it each time.
pageNode.classed('locked', !pageNode.classed('locked')); pageNode.classed('locked', !pageNode.classed('locked'));
}); });
}, },
......
...@@ -195,7 +195,7 @@ const ClassGraphPage = { ...@@ -195,7 +195,7 @@ const ClassGraphPage = {
this.displaySettingsData.readUrlProcessor(pageUrlProcessor); this.displaySettingsData.readUrlProcessor(pageUrlProcessor);
if (this.displaySettingsData.nodeFilterData.filterList.length === 0) { if (this.displaySettingsData.nodeFilterData.filterList.length === 0) {
// TODO(yjlong): This is test data. Remove this when no longer needed. // Default class to be displayed when the page is first loaded.
[ [
'org.chromium.chrome.browser.tab.TabImpl', 'org.chromium.chrome.browser.tab.TabImpl',
].forEach(nodeName => this.filterAddOrCheckNode(nodeName)); ].forEach(nodeName => this.filterAddOrCheckNode(nodeName));
......
...@@ -157,7 +157,7 @@ const PackageGraphPage = { ...@@ -157,7 +157,7 @@ const PackageGraphPage = {
this.displaySettingsData.readUrlProcessor(pageUrlProcessor); this.displaySettingsData.readUrlProcessor(pageUrlProcessor);
if (this.displaySettingsData.nodeFilterData.filterList.length === 0) { if (this.displaySettingsData.nodeFilterData.filterList.length === 0) {
// TODO(yjlong): This is test data. Remove this when no longer needed. // Default package to be displayed when the page is first loaded.
[ [
'org.chromium.chrome.browser.tab', 'org.chromium.chrome.browser.tab',
].forEach(nodeName => this.filterAddOrCheckNode(nodeName)); ].forEach(nodeName => this.filterAddOrCheckNode(nodeName));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment