Skip to content
This repository was archived by the owner on Sep 21, 2021. It is now read-only.

Commit 858a9a0

Browse files
committed
Don't inline SVG images in devtools-reps.
This causes warning in the browser console AND can cause performance issue. Using those images as background images means we can remove InlineSVG dependency and helpers. It also means we need to have a postCSS config to be able to transform those images URL for development in launchpad, and also for the copy-assets command.
1 parent 965628f commit 858a9a0

File tree

14 files changed

+185
-207
lines changed

14 files changed

+185
-207
lines changed

packages/devtools-reps/bin/copy-assets.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

55
const {
6-
tools: { makeBundle, symlinkTests, copyFile }
6+
tools: { makeBundle }
77
} = require("devtools-launchpad/index");
8-
const fs = require("fs-extra");
98
const path = require("path");
109
const minimist = require("minimist");
1110
const getConfig = require("./getConfig");
@@ -17,6 +16,7 @@ const args = minimist(process.argv.slice(2), {
1716

1817
function start() {
1918
console.log("start: copy assets");
19+
process.env.NODE_ENV = "production";
2020
const projectPath = path.resolve(__dirname, "..");
2121
const mcModulePath = "devtools/client/shared/components/reps";
2222

@@ -33,15 +33,13 @@ function start() {
3333
}
3434

3535
console.log(" output path is:", mcPath);
36-
3736
console.log(" creating reps.js bundle...");
37+
3838
makeBundle({
3939
outputPath: path.join(mcPath, mcModulePath),
4040
projectPath,
4141
watch: args.watch
4242
}).then(() => {
43-
console.log(" remove build artifacts...");
44-
fs.removeSync(path.join(mcPath, mcModulePath, "reps.js.map"));
4543
console.log("done: copy assets");
4644
}).catch(e => {
4745
console.error(e);

packages/devtools-reps/bin/dev-server.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,24 @@
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
44

5+
const path = require("path");
56
const toolbox = require("devtools-launchpad/index");
67
const feature = require("devtools-config");
78
const getConfig = require("./getConfig");
9+
const serve = require("express-static");
810

911
const envConfig = getConfig();
1012
feature.setConfig(envConfig);
1113

12-
const webpackConfig = require("../webpack.config");
13-
toolbox.startDevServer(envConfig, webpackConfig);
14+
let webpackConfig = require("../webpack.config");
15+
16+
let { app } = toolbox.startDevServer(envConfig, webpackConfig, __dirname);
17+
18+
// Serve devtools-reps images
19+
app.use("/devtools-reps/images/", serve(path.join(__dirname, "../src/shared/images")));
20+
21+
// As well as devtools-components ones, with a different path, which we are going to
22+
// write in the postCSS config in development mode.
23+
app.use("/devtools-components/images/",
24+
serve(path.join(__dirname, "../node_modules/devtools-components/src/images")));
25+

packages/devtools-reps/package.json

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,14 @@
2626
},
2727
"dependencies": {
2828
"classnames": "^2.2.5",
29-
"devtools-components": "^0.3.0",
29+
"devtools-components": "^0.4.1",
3030
"lodash": "^4.17.2",
3131
"prop-types": "^15.6.0",
3232
"react": "^16.2.0",
3333
"react-dom": "^16.2.0",
3434
"react-dom-factories": "^1.0.2",
3535
"react-redux": "^5.0.7",
36-
"redux": "^3.7.2",
37-
"svg-inline-react": "^3.0.0"
36+
"redux": "^3.7.2"
3837
},
3938
"devDependencies": {
4039
"@sucrase/webpack-object-rest-spread-plugin": "^1.0.0",
@@ -55,9 +54,9 @@
5554
"immutable": "^3.8.2",
5655
"jest": "^22.4.2",
5756
"jest-flow-transform": "^1.0.1",
57+
"postcss-url-mapper": "^1.2.0",
5858
"react-immutable-proptypes": "^2.1.0",
59-
"redux-logger": "=3.0.6",
60-
"svg-inline-loader": "^0.8.0"
59+
"redux-logger": "=3.0.6"
6160
},
6261
"jest": {
6362
"rootDir": "src",
@@ -81,8 +80,7 @@
8180
"node_modules/(?!devtools-)"
8281
],
8382
"moduleNameMapper": {
84-
"\\.css$": "<rootDir>/test/__mocks__/styleMock.js",
85-
"\\.svg$": "<rootDir>/test/__mocks__/svgMock.js"
83+
"\\.css$": "<rootDir>/test/__mocks__/styleMock.js"
8684
}
8785
}
8886
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/* This Source Code Form is subject to the terms of the Mozilla Public
2+
* License, v. 2.0. If a copy of the MPL was not distributed with this
3+
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4+
5+
const mapUrl = require("postcss-url-mapper");
6+
const MC_PATH = "chrome://devtools/skin/images/devtools-reps/";
7+
const EXPRESS_PATH = "/devtools-reps/images/";
8+
9+
function mapUrlProduction(url, type) {
10+
const newUrl = url
11+
.replace("/images/open-inspector.svg", MC_PATH + "open-inspector.svg")
12+
.replace("/images/jump-definition.svg", MC_PATH + "jump-definition.svg");
13+
14+
return newUrl;
15+
}
16+
17+
function mapUrlDevelopment(url) {
18+
const newUrl = url
19+
.replace("/images/open-inspector.svg", EXPRESS_PATH + "open-inspector.svg")
20+
.replace("/images/jump-definition.svg", EXPRESS_PATH + "jump-definition.svg");
21+
22+
return newUrl;
23+
}
24+
25+
module.exports = ({ file, options, env }) => {
26+
if (env === "production") {
27+
return {
28+
plugins: [mapUrl(mapUrlProduction)]
29+
};
30+
}
31+
32+
return {
33+
plugins: [mapUrl(mapUrlDevelopment)]
34+
};
35+
};

packages/devtools-reps/src/object-inspector/index.css

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
color: var(--theme-comment);
3737
}
3838

39-
.tree.object-inspector .arrow svg {
40-
pointer-events: none;
39+
.object-inspector .tree-node img.arrow {
40+
display: inline-block;
41+
vertical-align: middle;
4142
}

packages/devtools-reps/src/reps/element-node.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ const {
1313
const {rep: StringRep} = require("./string");
1414
const { MODE } = require("./constants");
1515
const nodeConstants = require("../shared/dom-node-constants");
16-
const Svg = require("../shared/images/Svg");
1716

1817
const dom = require("react-dom-factories");
1918
const { span } = dom;
@@ -61,9 +60,8 @@ function ElementNode(props) {
6160
}
6261

6362
if (onInspectIconClick) {
64-
inspectIcon = Svg("open-inspector", {
65-
element: "a",
66-
draggable: false,
63+
inspectIcon = dom.button({
64+
className: "open-inspector",
6765
// TODO: Localize this with "openNodeInInspector" when Bug 1317038 lands
6866
title: "Click to select the node in the inspector",
6967
onClick: (e) => onInspectIconClick(object, e)

packages/devtools-reps/src/reps/function.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ const {
1313
wrapRender,
1414
} = require("./rep-utils");
1515
const { MODE } = require("./constants");
16-
const Svg = require("../shared/images/Svg");
1716

1817
const dom = require("react-dom-factories");
1918
const { span } = dom;
@@ -41,8 +40,8 @@ function FunctionRep(props) {
4140
grip.location.url &&
4241
!IGNORED_SOURCE_URLS.includes(grip.location.url)
4342
) {
44-
jumpToDefinitionButton = Svg("jump-definition", {
45-
element: "a",
43+
jumpToDefinitionButton = dom.button({
44+
className: "jump-definition",
4645
draggable: false,
4746
title: "Jump to definition",
4847
onClick: e => {

packages/devtools-reps/src/reps/reps.css

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -230,35 +230,35 @@
230230
/******************************************************************************/
231231
/* Open DOMNode in inspector button */
232232

233-
.open-inspector svg {
234-
fill: var(--comment-node-color);
233+
button.open-inspector {
234+
mask: url(/images/open-inspector.svg) no-repeat;
235+
display: inline-block;
236+
background-color: var(--comment-node-color);
235237
height: 16px;
236-
width: 16px;
237238
margin-left: .25em;
238-
cursor: pointer;
239239
vertical-align: middle;
240240
}
241241

242-
.objectBox-node:hover .open-inspector svg,
243-
.objectBox-textNode:hover .open-inspector svg,
244-
.open-inspector svg:hover {
245-
fill: var(--theme-highlight-blue);
242+
.objectBox-node:hover .open-inspector,
243+
.objectBox-textNode:hover .open-inspector,
244+
.open-inspector:hover {
245+
background-color: var(--theme-highlight-blue);
246246
}
247247

248248
/******************************************************************************/
249249
/* Jump to definition button */
250250

251-
.jump-definition svg {
252-
stroke: var(--comment-node-color);
251+
button.jump-definition {
252+
mask: url(/images/jump-definition.svg) no-repeat;
253+
display: inline-block;
254+
background-color: var(--comment-node-color);
253255
height: 16px;
254-
width: 16px;
255256
margin-left: .25em;
256-
cursor: pointer;
257257
vertical-align: middle;
258258
}
259259

260-
.jump-definition svg:hover {
261-
stroke: var(--theme-highlight-blue);
260+
.jump-definition:hover {
261+
background-color: var(--theme-highlight-blue);
262262
}
263263

264264
/******************************************************************************/

packages/devtools-reps/src/reps/text-node.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ const {
1212
wrapRender,
1313
} = require("./rep-utils");
1414
const { MODE } = require("./constants");
15-
const Svg = require("../shared/images/Svg");
1615

1716
const dom = require("react-dom-factories");
1817
const { span } = dom;
@@ -59,8 +58,8 @@ function TextNode(props) {
5958
}
6059

6160
if (onInspectIconClick) {
62-
inspectIcon = Svg("open-inspector", {
63-
element: "a",
61+
inspectIcon = dom.button({
62+
className: "open-inspector",
6463
draggable: false,
6564
// TODO: Localize this with "openNodeInInspector" when Bug 1317038 lands
6665
title: "Click to select the node in the inspector",

packages/devtools-reps/src/shared/images/Svg.js

Lines changed: 0 additions & 36 deletions
This file was deleted.

0 commit comments

Comments
 (0)