From 8ed38f60e135d11ba30da113ebd8c58838667c33 Mon Sep 17 00:00:00 2001 From: silverwind Date: Mon, 5 Apr 2021 22:25:36 +0200 Subject: [PATCH] Enable JSX support in frontend This enables frontend to use JSX to generate DOM nodes using the `jsx-dom` module [1]. JSX is inherently safer than concatenating HTML strings because it defaults to HTML escaping. Also it is easier to work with and more powerful than template strings. To demonstrate JSX usage, I've rewritten the context popup feature to use it, no functional changes there. [1]: https://github.com/proteriax/jsx-dom --- .eslintrc | 2 + package-lock.json | 27 ++++++++++++++ package.json | 1 + web_src/js/components.js | 17 +++++++++ web_src/js/features/contextpopup.js | 58 +++++++++++++---------------- web_src/js/svg.js | 22 +++++++---- web_src/js/utils.js | 10 +++++ web_src/js/utils.test.js | 13 ++++++- webpack.config.js | 11 +++++- 9 files changed, 119 insertions(+), 42 deletions(-) create mode 100644 web_src/js/components.js diff --git a/.eslintrc b/.eslintrc index 640956f73424f..90c07d5d750ce 100644 --- a/.eslintrc +++ b/.eslintrc @@ -9,6 +9,8 @@ ignorePatterns: parserOptions: sourceType: module ecmaVersion: 2021 + ecmaFeatures: + jsx: true plugins: - eslint-plugin-unicorn diff --git a/package-lock.json b/package-lock.json index 0ec6f4345605c..94a9d7a439209 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,6 +20,7 @@ "font-awesome": "4.7.0", "jquery": "3.6.0", "jquery.are-you-sure": "1.9.0", + "jsx-dom": "7.0.0-beta.5", "less": "4.1.1", "less-loader": "8.1.1", "license-checker-webpack-plugin": "0.2.1", @@ -3044,6 +3045,11 @@ "integrity": "sha512-b0tGHbfegbhPJpxpiBPU2sCkigAqtM9O121le6bbOlgyV+NyGyCmVfJ6QW9eRjz8CpNfWEOYBIMIGRYkLwsIYg==", "dev": true }, + "node_modules/csstype": { + "version": "3.0.8", + "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.0.8.tgz", + "integrity": "sha512-jXKhWqXPmlUeoQnF/EhTtTl4C9SnrxSH/jZUih3jmO6lBKr99rP3/+FmrMj4EFpOXzMtXHAZkd3x0E6h6Fgflw==" + }, "node_modules/d3": { "version": "5.16.0", "resolved": "https://registry.npmjs.org/d3/-/d3-5.16.0.tgz", @@ -7498,6 +7504,14 @@ "verror": "1.10.0" } }, + "node_modules/jsx-dom": { + "version": "7.0.0-beta.5", + "resolved": "https://registry.npmjs.org/jsx-dom/-/jsx-dom-7.0.0-beta.5.tgz", + "integrity": "sha512-SJRvQmFwVItkI3/pXXZ3piW07hLCSbrA5hGTxpsbAKoZ2VFl0j8QFwgSytdq3F7VsoC9TWZWheIpk8qT2ZecpQ==", + "dependencies": { + "csstype": "^3.0.7" + } + }, "node_modules/khroma": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/khroma/-/khroma-1.4.1.tgz", @@ -16022,6 +16036,11 @@ } } }, + "csstype": { + "version": "3.0.8", + "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.0.8.tgz", + "integrity": "sha512-jXKhWqXPmlUeoQnF/EhTtTl4C9SnrxSH/jZUih3jmO6lBKr99rP3/+FmrMj4EFpOXzMtXHAZkd3x0E6h6Fgflw==" + }, "d3": { "version": "5.16.0", "resolved": "https://registry.npmjs.org/d3/-/d3-5.16.0.tgz", @@ -19506,6 +19525,14 @@ "verror": "1.10.0" } }, + "jsx-dom": { + "version": "7.0.0-beta.5", + "resolved": "https://registry.npmjs.org/jsx-dom/-/jsx-dom-7.0.0-beta.5.tgz", + "integrity": "sha512-SJRvQmFwVItkI3/pXXZ3piW07hLCSbrA5hGTxpsbAKoZ2VFl0j8QFwgSytdq3F7VsoC9TWZWheIpk8qT2ZecpQ==", + "requires": { + "csstype": "^3.0.7" + } + }, "khroma": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/khroma/-/khroma-1.4.1.tgz", diff --git a/package.json b/package.json index 5da3dfab20800..4eda9ed69dfcf 100644 --- a/package.json +++ b/package.json @@ -20,6 +20,7 @@ "font-awesome": "4.7.0", "jquery": "3.6.0", "jquery.are-you-sure": "1.9.0", + "jsx-dom": "7.0.0-beta.5", "less": "4.1.1", "less-loader": "8.1.1", "license-checker-webpack-plugin": "0.2.1", diff --git a/web_src/js/components.js b/web_src/js/components.js new file mode 100644 index 0000000000000..38e533f91bc0c --- /dev/null +++ b/web_src/js/components.js @@ -0,0 +1,17 @@ +import {contrastColor} from './utils.js'; + +// These components might look like React components but they are +// not. They return DOM nodes via JSX transformation using jsx-dom. +// https://github.com/proteriax/jsx-dom + +export function Label({label}) { + const backgroundColor = `#${label.color}`; + const color = contrastColor(backgroundColor); + const style = `color: ${color}; background-color: ${backgroundColor}`; + + return ( +
+ {label.name} +
+ ); +} diff --git a/web_src/js/features/contextpopup.js b/web_src/js/features/contextpopup.js index c16820cf1f740..5e28caff190fc 100644 --- a/web_src/js/features/contextpopup.js +++ b/web_src/js/features/contextpopup.js @@ -1,5 +1,5 @@ -import {htmlEscape} from 'escape-goat'; -import {svg} from '../svg.js'; +import {Label} from '../components.js'; +import {SVG} from '../svg.js'; const {AppSubUrl} = window.config; @@ -22,40 +22,24 @@ function issuePopup(owner, repo, index, $element) { body = `${body.substring(0, 85)}...`; } - let labels = ''; - for (let i = 0; i < issue.labels.length; i++) { - const label = issue.labels[i]; - const red = parseInt(label.color.substring(0, 2), 16); - const green = parseInt(label.color.substring(2, 4), 16); - const blue = parseInt(label.color.substring(4, 6), 16); - let color = '#ffffff'; - if ((red * 0.299 + green * 0.587 + blue * 0.114) > 125) { - color = '#000000'; - } - labels += `
${htmlEscape(label.name)}
`; - } - if (labels.length > 0) { - labels = `

${labels}

`; - } - - let octicon, color; + let icon, color; if (issue.pull_request !== null) { if (issue.state === 'open') { color = 'green'; - octicon = 'octicon-git-pull-request'; // Open PR + icon = 'octicon-git-pull-request'; // Open PR } else if (issue.pull_request.merged === true) { color = 'purple'; - octicon = 'octicon-git-merge'; // Merged PR + icon = 'octicon-git-merge'; // Merged PR } else { color = 'red'; - octicon = 'octicon-git-pull-request'; // Closed PR + icon = 'octicon-git-pull-request'; // Closed PR } } else if (issue.state === 'open') { color = 'green'; - octicon = 'octicon-issue-opened'; // Open Issue + icon = 'octicon-issue-opened'; // Open Issue } else { color = 'red'; - octicon = 'octicon-issue-closed'; // Closed Issue + icon = 'octicon-issue-closed'; // Closed Issue } $element.popup({ @@ -63,14 +47,24 @@ function issuePopup(owner, repo, index, $element) { delay: { show: 250 }, - html: ` -
-

${htmlEscape(issue.repository.full_name)} on ${createdAt}

-

${svg(octicon)} ${htmlEscape(issue.title)} #${index}

-

${htmlEscape(body)}

- ${labels} -
-` + html: ( +
+

{issue.repository.full_name} on {createdAt}

+

+ + {issue.title} + #{index} +

+

{body}

+ {issue.labels && issue.labels.length && ( +

+ {issue.labels.map((label) => ( +

+ )} +
+ ) }); }); } diff --git a/web_src/js/svg.js b/web_src/js/svg.js index 0960256e2121c..f57139b032cc4 100644 --- a/web_src/js/svg.js +++ b/web_src/js/svg.js @@ -35,15 +35,23 @@ export const svgs = { const parser = new DOMParser(); const serializer = new XMLSerializer(); -// retrieve a HTML string for given SVG icon name, size and additional classes -export function svg(name, size = 16, className = '') { - if (!(name in svgs)) return ''; - if (size === 16 && !className) return svgs[name]; +// returns DOM node for given SVG icon name, size and additional classes +export function SVG({name, size = 16, className = ''}) { + if (!(name in svgs)) return null; - const document = parser.parseFromString(svgs[name], 'image/svg+xml'); - const svgNode = document.firstChild; + // parse as html to avoid namespace issues + const document = parser.parseFromString(svgs[name], 'text/html'); + const svgNode = document.body.firstChild; if (size !== 16) svgNode.setAttribute('width', String(size)); if (size !== 16) svgNode.setAttribute('height', String(size)); if (className) svgNode.classList.add(...className.split(/\s+/)); - return serializer.serializeToString(svgNode); + return svgNode; +} + +// returns a HTML string for given SVG icon name, size and additional classes +export function svg(name, size = 16, className = '') { + if (!(name in svgs)) return ''; + if (size === 16 && !className) return svgs[name]; + const svgElement = ; + return serializer.serializeToString(svgElement); } diff --git a/web_src/js/utils.js b/web_src/js/utils.js index 58d0d6b39ec22..fed88c5b7927e 100644 --- a/web_src/js/utils.js +++ b/web_src/js/utils.js @@ -51,3 +51,13 @@ export function mqBinarySearch(feature, minValue, maxValue, step, unit) { } return mqBinarySearch(feature, minValue, mid - step, step, unit); // feature is < mid } + +// get a contrasting foreground color for a given 6-digit background color +export function contrastColor(hex) { + const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex); + if (!result) return '#fff'; + const r = parseInt(result[1], 16); + const g = parseInt(result[2], 16); + const b = parseInt(result[3], 16); + return ((r * 299) + (g * 587) + (b * 114)) / 1000 > 125 ? '#000' : '#fff'; +} diff --git a/web_src/js/utils.test.js b/web_src/js/utils.test.js index 859046c87c976..b1ee5f65e930c 100644 --- a/web_src/js/utils.test.js +++ b/web_src/js/utils.test.js @@ -1,5 +1,5 @@ import { - basename, extname, isObject, uniq, stripTags, joinPaths, + basename, extname, isObject, uniq, stripTags, joinPaths, contrastColor, } from './utils.js'; test('basename', () => { @@ -66,3 +66,14 @@ test('uniq', () => { test('stripTags', () => { expect(stripTags('test')).toEqual('test'); }); + +test('contrastColor', () => { + expect(contrastColor('#000000')).toEqual('#fff'); + expect(contrastColor('#333333')).toEqual('#fff'); + expect(contrastColor('#ff0000')).toEqual('#fff'); + expect(contrastColor('#0000ff')).toEqual('#fff'); + expect(contrastColor('#cccccc')).toEqual('#000'); + expect(contrastColor('#ffffff')).toEqual('#000'); + expect(contrastColor('000000')).toEqual('#fff'); + expect(contrastColor('ffffff')).toEqual('#000'); +}); diff --git a/webpack.config.js b/webpack.config.js index 9b27bb55ac64f..453ea8fb31eee 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -12,7 +12,7 @@ import {fileURLToPath} from 'url'; const {VueLoaderPlugin} = VueLoader; const {ESBuildMinifyPlugin} = EsBuildLoader; -const {SourceMapDevToolPlugin} = webpack; +const {SourceMapDevToolPlugin, ProvidePlugin} = webpack; const __dirname = dirname(fileURLToPath(import.meta.url)); const glob = (pattern) => fastGlob.sync(pattern, {cwd: __dirname, absolute: true}); @@ -122,7 +122,10 @@ export default { { loader: 'esbuild-loader', options: { - target: 'es2015' + target: 'es2015', + loader: 'jsx', + jsxFactory: 'h', + jsxFragment: 'Fragment', }, }, ], @@ -205,6 +208,10 @@ export default { new MonacoWebpackPlugin({ filename: 'js/monaco-[name].worker.js', }), + new ProvidePlugin({ + h: ['jsx-dom', 'h'], + Fragment: ['jsx-dom', 'Fragment'], + }), isProduction ? new LicenseCheckerWebpackPlugin({ outputFilename: 'js/licenses.txt', outputWriter: ({dependencies}) => {