From 8b577ca12fafeeba308576d9fddb8987a28d169a Mon Sep 17 00:00:00 2001 From: Benedict Aas Date: Tue, 20 Feb 2018 09:51:13 +0000 Subject: [PATCH] feat(GUI): separate svg path and content attributes (#1677) We separate the SVG component path and content into attributes `paths` and `contents` which take lists of strings that are tried until one succeeds. `contents` takes precedence over `paths`, i.e. it is tried first. Change-Type: patch Changelog-Entry: Separate SVG component's path and content attributes. --- lib/gui/app/components/svg-icon.js | 88 +++++++++++++++---- lib/gui/app/index.html | 4 +- .../pages/finish/templates/success.tpl.html | 6 +- .../app/pages/main/templates/main.tpl.html | 6 +- tests/gui/components/svg-icon.spec.js | 60 ++++++++++--- 5 files changed, 123 insertions(+), 41 deletions(-) diff --git a/lib/gui/app/components/svg-icon.js b/lib/gui/app/components/svg-icon.js index b7f70a6a..3398d5cc 100644 --- a/lib/gui/app/components/svg-icon.js +++ b/lib/gui/app/components/svg-icon.js @@ -29,6 +29,7 @@ const propTypes = require('prop-types') const react2angular = require('react2angular').react2angular const path = require('path') const fs = require('fs') +const analytics = require('../modules/analytics') const MODULE_NAME = 'Etcher.Components.SVGIcon' const angularSVGIcon = angular.module(MODULE_NAME, []) @@ -37,6 +38,29 @@ const DEFAULT_SIZE = '40px' const domParser = new window.DOMParser() +/** + * @summary Try to parse SVG contents and return it data encoded + * + * @param {String} contents - SVG XML contents + * @returns {String|null} + * + * @example + * const encodedSVG = tryParseSVGContents('') + * + * img.src = encodedSVG + */ +const tryParseSVGContents = (contents) => { + const doc = domParser.parseFromString(contents, 'image/svg+xml') + const parserError = doc.querySelector('parsererror') + const svg = doc.querySelector('svg') + + if (!parserError && svg) { + return `data:image/svg+xml,${encodeURIComponent(svg.outerHTML)}` + } + + return null +} + /** * @summary SVG element that takes both filepaths and file contents * @type {Object} @@ -59,31 +83,52 @@ class SVGIcon extends react.Component { // eslint-disable-next-line no-underscore-dangle : global.__dirname - // This means the path to the icon should be - // relative to *this directory*. - // TODO: There might be a way to compute the path - // relatively to the `index.html`. - const imagePath = path.join(baseDirectory, 'assets', this.props.path) + let svgData = '' - let contents = '' + _.find(this.props.contents, (content) => { + const attempt = tryParseSVGContents(content) - if (_.startsWith(this.props.path, '<')) { - contents = this.props.path - } else { - contents = fs.readFileSync(imagePath, { - encoding: 'utf8' + if (attempt) { + svgData = attempt + return true + } + + return false + }) + + if (!svgData) { + _.find(this.props.paths, (relativePath) => { + // This means the path to the icon should be + // relative to *this directory*. + // TODO: There might be a way to compute the path + // relatively to the `index.html`. + const imagePath = path.join(baseDirectory, 'assets', relativePath) + + const contents = _.attempt(() => { + return fs.readFileSync(imagePath, { + encoding: 'utf8' + }) + }) + + if (_.isError(contents)) { + analytics.logException(contents) + return false + } + + const parsed = _.attempt(tryParseSVGContents, contents) + + if (parsed) { + svgData = parsed + return true + } + + return false }) } const width = this.props.width || DEFAULT_SIZE const height = this.props.height || DEFAULT_SIZE - const doc = domParser.parseFromString(contents, 'image/svg+xml') - const parserError = doc.querySelector('parsererror') - const svg = doc.querySelector('svg') - const svgXml = svg && _.isNil(parserError) ? svg.outerHTML : '' - const svgData = `data:image/svg+xml,${encodeURIComponent(svgXml)}` - return react.createElement('img', { className: 'svg-icon', style: { @@ -108,9 +153,14 @@ class SVGIcon extends react.Component { SVGIcon.propTypes = { /** - * @summary SVG contents or path to the resource + * @summary Paths to SVG files to be tried in succession if any fails */ - path: propTypes.string.isRequired, + paths: propTypes.array, + + /** + * @summary List of embedded SVG contents to be tried in succession if any fails + */ + contents: propTypes.array, /** * @summary SVG image width unit diff --git a/lib/gui/app/index.html b/lib/gui/app/index.html index 1f9c1a68..3eba3e61 100644 --- a/lib/gui/app/index.html +++ b/lib/gui/app/index.html @@ -38,7 +38,7 @@ ng-hide="state.currentName === 'success'"> - @@ -51,7 +51,7 @@ - diff --git a/lib/gui/app/pages/finish/templates/success.tpl.html b/lib/gui/app/pages/finish/templates/success.tpl.html index 953ee6ab..ff2750ff 100644 --- a/lib/gui/app/pages/finish/templates/success.tpl.html +++ b/lib/gui/app/pages/finish/templates/success.tpl.html @@ -16,7 +16,7 @@
Thanks for using - @@ -24,12 +24,12 @@
made with - by - diff --git a/lib/gui/app/pages/main/templates/main.tpl.html b/lib/gui/app/pages/main/templates/main.tpl.html index e69ecfa0..99cc5474 100644 --- a/lib/gui/app/pages/main/templates/main.tpl.html +++ b/lib/gui/app/pages/main/templates/main.tpl.html @@ -3,7 +3,7 @@
- +
@@ -44,7 +44,7 @@
-
@@ -87,7 +87,7 @@
-
diff --git a/tests/gui/components/svg-icon.spec.js b/tests/gui/components/svg-icon.spec.js index 94c31e86..e24621a6 100644 --- a/tests/gui/components/svg-icon.spec.js +++ b/tests/gui/components/svg-icon.spec.js @@ -35,11 +35,12 @@ describe('Browser: SVGIcon', function () { beforeEach(angular.mock.inject(function (_$compile_, _$rootScope_) { $compile = _$compile_ $rootScope = _$rootScope_ + + this.iconPath = '../../../lib/gui/assets/etcher.svg' })) it('should inline the svg contents in the element', function () { - const icon = '../../../gui/assets/etcher.svg' - let iconContents = _.split(fs.readFileSync(path.join(__dirname, '../../../lib/gui/assets/etcher.svg'), { + let iconContents = _.split(fs.readFileSync(path.join(__dirname, this.iconPath), { encoding: 'utf8' }), /\r?\n/) @@ -48,7 +49,7 @@ describe('Browser: SVGIcon', function () { iconContents[0] = `` iconContents = iconContents.join('\n') - const element = $compile(`Resin.io`)($rootScope) + const element = $compile(`Resin.io`)($rootScope) $rootScope.$digest() // We parse the SVGs to get rid of discrepancies caused by string differences @@ -62,12 +63,47 @@ describe('Browser: SVGIcon', function () { m.chai.expect(compiledDoc.outerHTML).to.equal(originalDoc.outerHTML) }) - it('should accept an SVG in the path attribute', function () { + it('should try next path if previous was not found', function () { + let iconContents = _.split(fs.readFileSync(path.join(__dirname, this.iconPath), { + encoding: 'utf8' + }), /\r?\n/) + + // Injecting XML as HTML causes the XML header to be commented out. + // Modify here to ease assertions later on. + iconContents[0] = `` + iconContents = iconContents.join('\n') + + const element = $compile(`Resin.io`)($rootScope) + $rootScope.$digest() + + // We parse the SVGs to get rid of discrepancies caused by string differences + // in the outputs; the XML trees are still equal, as proven here. + const originalSVGParser = new DOMParser() + const originalDoc = originalSVGParser.parseFromString(iconContents, 'image/svg+xml') + const compiledSVGParser = new DOMParser() + const compiledContents = decodeURIComponent(element.children()[0].src.substr(19)) + const compiledDoc = compiledSVGParser.parseFromString(compiledContents, 'image/svg+xml') + + m.chai.expect(compiledDoc.outerHTML).to.equal(originalDoc.outerHTML) + }) + + it('should accept an SVG in the contents attribute', function () { const iconContents = '' const imgData = `data:image/svg+xml,${encodeURIComponent(iconContents)}` $rootScope.iconContents = iconContents - const element = $compile('Resin.io')($rootScope) + const element = $compile('Resin.io')($rootScope) + $rootScope.$digest() + m.chai.expect(element.children().attr('src')).to.equal(imgData) + }) + + it('should prioritise the contents attribute over the paths attribute', function () { + const iconContents = '' + const imgData = `data:image/svg+xml,${encodeURIComponent(iconContents)}` + $rootScope.iconContents = iconContents + + const svg = `Resin.io` + const element = $compile(svg)($rootScope) $rootScope.$digest() m.chai.expect(element.children().attr('src')).to.equal(imgData) }) @@ -75,33 +111,29 @@ describe('Browser: SVGIcon', function () { it('should use an empty src if there is a parsererror', function () { // The following is invalid, because there's no closing tag for `foreignObject` const iconContents = '' - const imgData = 'data:image/svg+xml,' $rootScope.iconContents = iconContents - const element = $compile('Resin.io')($rootScope) + const element = $compile('Resin.io')($rootScope) $rootScope.$digest() - m.chai.expect(element.children().attr('src')).to.equal(imgData) + m.chai.expect(element.children().attr('src')).to.be.empty }) it('should default the size to 40x40 pixels', function () { - const icon = '../../../gui/assets/etcher.svg' - const element = $compile(`Resin.io`)($rootScope) + const element = $compile(`Resin.io`)($rootScope) $rootScope.$digest() m.chai.expect(element.children().css('width')).to.equal('40px') m.chai.expect(element.children().css('height')).to.equal('40px') }) it('should be able to set a custom width', function () { - const icon = '../../../gui/assets/etcher.svg' - const element = $compile(`Resin.io`)($rootScope) + const element = $compile(`Resin.io`)($rootScope) $rootScope.$digest() m.chai.expect(element.children().css('width')).to.equal('20px') m.chai.expect(element.children().css('height')).to.equal('40px') }) it('should be able to set a custom height', function () { - const icon = '../../../gui/assets/etcher.svg' - const element = $compile(`Resin.io`)($rootScope) + const element = $compile(`Resin.io`)($rootScope) $rootScope.$digest() m.chai.expect(element.children().css('width')).to.equal('40px') m.chai.expect(element.children().css('height')).to.equal('20px')