From 6d93d554bd408a66767c415320f4d1a53c74572b Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 19 Mar 2023 13:29:51 +0800 Subject: [PATCH 1/7] fix --- web_src/js/svg.js | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/web_src/js/svg.js b/web_src/js/svg.js index e431ca57e643e..11385396829fe 100644 --- a/web_src/js/svg.js +++ b/web_src/js/svg.js @@ -107,7 +107,6 @@ export function svg(name, size = 16, className = '') { const svgNode = document.firstChild; if (size !== 16) svgNode.setAttribute('width', String(size)); if (size !== 16) svgNode.setAttribute('height', String(size)); - // filter array to remove empty string if (className) svgNode.classList.add(...className.split(/\s+/).filter(Boolean)); return serializer.serializeToString(svgNode); } @@ -120,6 +119,45 @@ export const SvgIcon = { className: {type: String, default: ''}, }, render() { - return h('span', {innerHTML: svg(this.name, this.size, this.className)}); + const svgStr = svgs[this.name]; + if (!svgStr) throw new Error(`Unknown SVG icon: ${this.name}`); + + // parse the SVG string to 2 parts + // * svgInnerHtml: the inner part of the SVG, will be used as the content of the VNode + // * svgOuter: the outer part of the SVG, including attributes + // the builtin SVG contents are clean, so it's safe to use `indexOf` to split the content: + // eg: ${svgInnerHtml} + const p1 = svgStr.indexOf('>'), p2 = svgStr.lastIndexOf('<'); + if (p1 === -1 || p2 === -1) throw new Error(`Invalid SVG icon: ${this.name}`); + const svgInnerHtml = svgStr.slice(p1 + 1, p2); + const svgOuterHtml = svgStr.slice(0, p1 + 1) + svgStr.slice(p2); + const svgDoc = parser.parseFromString(svgOuterHtml, 'image/svg+xml'); + const svgOuter = svgDoc.firstChild; + + // https://vuejs.org/guide/extras/render-function.html#creating-vnodes + // the `^` is used for attr, set SVG attributes like 'width', `aria-hidden`, `viewBox`, etc + const attrs = {}; + for (const attr of svgOuter.attributes) { + if (attr.name === 'class') continue; + attrs[`^${attr.name}`] = attr.value; + } + attrs[`^width`] = this.size; + attrs[`^height`] = this.size; + + // make the classes work together + const classes = []; + for (const cls of svgOuter.classList) { + classes.push(cls); + } + if (this.className) { + classes.push(...this.className.split(/\s+/).filter(Boolean)); + } + + // create VNode + return h('svg', { + ...attrs, + class: classes, + innerHTML: svgInnerHtml, + }); }, }; From 367ffca1961a447d692406617ddcc09a69f0eb12 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Tue, 21 Mar 2023 15:21:30 +0800 Subject: [PATCH 2/7] remove unnecessary "span.color .svg {}" --- web_src/css/base.css | 12 ------------ web_src/js/components/ActionRunStatus.vue | 4 ++-- web_src/js/components/ContextPopup.vue | 2 +- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/web_src/css/base.css b/web_src/css/base.css index 1077a0eebd022..e3b3fece377b5 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -2420,18 +2420,6 @@ a.ui.basic.label:hover { height: 2.1666em !important; } -span.green .svg { - color: var(--color-green); -} - -span.red .svg { - color: var(--color-red); -} - -span.purple .svg { - color: var(--color-purple); -} - .migrate .svg.gitea-git { color: var(--color-git); } diff --git a/web_src/js/components/ActionRunStatus.vue b/web_src/js/components/ActionRunStatus.vue index 3717b64956617..b72dfb1aa699e 100644 --- a/web_src/js/components/ActionRunStatus.vue +++ b/web_src/js/components/ActionRunStatus.vue @@ -1,10 +1,10 @@

{{ issue.repository.full_name }} on {{ createdAt }}

-

{{ issue.title }} #{{ issue.number }}

+

{{ issue.title }} #{{ issue.number }}

{{ body }}

Date: Tue, 21 Mar 2023 15:32:49 +0800 Subject: [PATCH 3/7] fix incorrect color usage --- templates/projects/view.tmpl | 2 +- templates/repo/projects/view.tmpl | 2 +- templates/user/settings/keys_gpg.tmpl | 2 +- templates/user/settings/keys_ssh.tmpl | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/templates/projects/view.tmpl b/templates/projects/view.tmpl index b776f89efa39c..b778e0a242d67 100644 --- a/templates/projects/view.tmpl +++ b/templates/projects/view.tmpl @@ -224,7 +224,7 @@ {{- range index $.LinkedPRs .ID}}
- {{svg "octicon-git-merge" 16 "gt-mr-2 gt-vm"}} + {{svg "octicon-git-merge" 16 "gt-mr-2 gt-vm"}} {{.Title}} #{{.Index}}
diff --git a/templates/repo/projects/view.tmpl b/templates/repo/projects/view.tmpl index 0248b9c6d2c7a..99831f3dd9c33 100644 --- a/templates/repo/projects/view.tmpl +++ b/templates/repo/projects/view.tmpl @@ -235,7 +235,7 @@ {{- range index $.LinkedPRs .ID}}
- {{svg "octicon-git-merge" 16 "gt-mr-2 gt-vm"}} + {{svg "octicon-git-merge" 16 "gt-mr-2 gt-vm"}} {{.Title}} #{{.Index}}
diff --git a/templates/user/settings/keys_gpg.tmpl b/templates/user/settings/keys_gpg.tmpl index ddb4960642d50..d143d5ed99721 100644 --- a/templates/user/settings/keys_gpg.tmpl +++ b/templates/user/settings/keys_gpg.tmpl @@ -54,7 +54,7 @@ {{end}}
- {{svg "octicon-key" 32}} + {{svg "octicon-key" 32}}
{{if .Verified}} diff --git a/templates/user/settings/keys_ssh.tmpl b/templates/user/settings/keys_ssh.tmpl index 262b6e0d739ce..71b9b6a86b68b 100644 --- a/templates/user/settings/keys_ssh.tmpl +++ b/templates/user/settings/keys_ssh.tmpl @@ -47,7 +47,7 @@
- {{svg "octicon-key" 32}} + {{svg "octicon-key" 32}}
{{if .Verified}} From 5677a8128d2217300e9e164b997e6c464a03d1e0 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 22 Mar 2023 23:18:13 +0800 Subject: [PATCH 4/7] Update web_src/js/svg.js, fix lint Co-authored-by: silverwind --- web_src/js/svg.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/web_src/js/svg.js b/web_src/js/svg.js index 11385396829fe..44a0fd527a494 100644 --- a/web_src/js/svg.js +++ b/web_src/js/svg.js @@ -105,8 +105,10 @@ export function svg(name, size = 16, className = '') { const document = parser.parseFromString(svgs[name], 'image/svg+xml'); const svgNode = document.firstChild; - if (size !== 16) svgNode.setAttribute('width', String(size)); - if (size !== 16) svgNode.setAttribute('height', String(size)); + if (size !== 16) { + svgNode.setAttribute('width', String(size)); + svgNode.setAttribute('height', String(size)); + } if (className) svgNode.classList.add(...className.split(/\s+/).filter(Boolean)); return serializer.serializeToString(svgNode); } From d30cc25d049ab4c113f44cf276e141d7197dbc8e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 22 Mar 2023 23:37:58 +0800 Subject: [PATCH 5/7] tests --- web_src/js/svg.js | 35 ++++++++++++++++++++--------------- web_src/js/svg.test.js | 9 ++++++++- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/web_src/js/svg.js b/web_src/js/svg.js index 44a0fd527a494..6fa79b8e9b3a1 100644 --- a/web_src/js/svg.js +++ b/web_src/js/svg.js @@ -113,6 +113,24 @@ export function svg(name, size = 16, className = '') { return serializer.serializeToString(svgNode); } +export function svgParseOuterInner(name) { + const svgStr = svgs[name]; + if (!svgStr) throw new Error(`Unknown SVG icon: ${name}`); + + // parse the SVG string to 2 parts + // * svgInnerHtml: the inner part of the SVG, will be used as the content of the VNode + // * svgOuter: the outer part of the SVG, including attributes + // the builtin SVG contents are clean, so it's safe to use `indexOf` to split the content: + // eg: ${svgInnerHtml} + const p1 = svgStr.indexOf('>'), p2 = svgStr.lastIndexOf('<'); + if (p1 === -1 || p2 === -1) throw new Error(`Invalid SVG icon: ${name}`); + const svgInnerHtml = svgStr.slice(p1 + 1, p2); + const svgOuterHtml = svgStr.slice(0, p1 + 1) + svgStr.slice(p2); + const svgDoc = parser.parseFromString(svgOuterHtml, 'image/svg+xml'); + const svgOuter = svgDoc.firstChild; + return {svgOuter, svgInnerHtml}; +} + export const SvgIcon = { name: 'SvgIcon', props: { @@ -121,21 +139,7 @@ export const SvgIcon = { className: {type: String, default: ''}, }, render() { - const svgStr = svgs[this.name]; - if (!svgStr) throw new Error(`Unknown SVG icon: ${this.name}`); - - // parse the SVG string to 2 parts - // * svgInnerHtml: the inner part of the SVG, will be used as the content of the VNode - // * svgOuter: the outer part of the SVG, including attributes - // the builtin SVG contents are clean, so it's safe to use `indexOf` to split the content: - // eg: ${svgInnerHtml} - const p1 = svgStr.indexOf('>'), p2 = svgStr.lastIndexOf('<'); - if (p1 === -1 || p2 === -1) throw new Error(`Invalid SVG icon: ${this.name}`); - const svgInnerHtml = svgStr.slice(p1 + 1, p2); - const svgOuterHtml = svgStr.slice(0, p1 + 1) + svgStr.slice(p2); - const svgDoc = parser.parseFromString(svgOuterHtml, 'image/svg+xml'); - const svgOuter = svgDoc.firstChild; - + const {svgOuter, svgInnerHtml} = svgParseOuterInner(this.name); // https://vuejs.org/guide/extras/render-function.html#creating-vnodes // the `^` is used for attr, set SVG attributes like 'width', `aria-hidden`, `viewBox`, etc const attrs = {}; @@ -151,6 +155,7 @@ export const SvgIcon = { for (const cls of svgOuter.classList) { classes.push(cls); } + // TODO: drop the `className/class-name` prop in the future, only use "class" prop if (this.className) { classes.push(...this.className.split(/\s+/).filter(Boolean)); } diff --git a/web_src/js/svg.test.js b/web_src/js/svg.test.js index c5d6d07535c7b..8366ab5ac7d96 100644 --- a/web_src/js/svg.test.js +++ b/web_src/js/svg.test.js @@ -1,8 +1,15 @@ import {expect, test} from 'vitest'; -import {svg} from './svg.js'; +import {svg, svgParseOuterInner} from './svg.js'; test('svg', () => { expect(svg('octicon-repo')).toMatch(/^ { + const {svgOuter, svgInnerHtml} = svgParseOuterInner('octicon-repo'); + expect(svgOuter.nodeName).toMatch('svg'); + expect(svgOuter.classList.contains('octicon-repo')).toBeTruthy(); + expect(svgInnerHtml).toContain(' Date: Wed, 22 Mar 2023 23:59:28 +0800 Subject: [PATCH 6/7] add tests for Vue --- web_src/js/svg.test.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/web_src/js/svg.test.js b/web_src/js/svg.test.js index 8366ab5ac7d96..f110f31f878d4 100644 --- a/web_src/js/svg.test.js +++ b/web_src/js/svg.test.js @@ -1,5 +1,6 @@ import {expect, test} from 'vitest'; -import {svg, svgParseOuterInner} from './svg.js'; +import {svg, SvgIcon, svgParseOuterInner} from './svg.js'; +import {createApp, h} from 'vue'; test('svg', () => { expect(svg('octicon-repo')).toMatch(/^ { expect(svgOuter.nodeName).toMatch('svg'); expect(svgOuter.classList.contains('octicon-repo')).toBeTruthy(); expect(svgInnerHtml).toContain(' { + const root = document.createElement('div'); + createApp({render: () => h(SvgIcon, {name: 'octicon-link', size: 24, class: 'base', className: 'extra'})}).mount(root); + expect(root.firstChild.nodeName).toEqual('svg'); + expect(root.firstChild.getAttribute('width')).toEqual('24'); + expect(root.firstChild.getAttribute('height')).toEqual('24'); + expect(root.firstChild.classList.contains('octicon-link')).toBeTruthy(); + expect(root.firstChild.classList.contains('base')).toBeTruthy(); + expect(root.firstChild.classList.contains('extra')).toBeTruthy(); +}); From 6564b5df509070a299edc9fb2f42cb117ef63004 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 23 Mar 2023 00:05:55 +0800 Subject: [PATCH 7/7] use variable for test --- web_src/js/svg.test.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/web_src/js/svg.test.js b/web_src/js/svg.test.js index f110f31f878d4..5db2f65ac8d68 100644 --- a/web_src/js/svg.test.js +++ b/web_src/js/svg.test.js @@ -18,10 +18,11 @@ test('svgParseOuterInner', () => { test('SvgIcon', () => { const root = document.createElement('div'); createApp({render: () => h(SvgIcon, {name: 'octicon-link', size: 24, class: 'base', className: 'extra'})}).mount(root); - expect(root.firstChild.nodeName).toEqual('svg'); - expect(root.firstChild.getAttribute('width')).toEqual('24'); - expect(root.firstChild.getAttribute('height')).toEqual('24'); - expect(root.firstChild.classList.contains('octicon-link')).toBeTruthy(); - expect(root.firstChild.classList.contains('base')).toBeTruthy(); - expect(root.firstChild.classList.contains('extra')).toBeTruthy(); + const node = root.firstChild; + expect(node.nodeName).toEqual('svg'); + expect(node.getAttribute('width')).toEqual('24'); + expect(node.getAttribute('height')).toEqual('24'); + expect(node.classList.contains('octicon-link')).toBeTruthy(); + expect(node.classList.contains('base')).toBeTruthy(); + expect(node.classList.contains('extra')).toBeTruthy(); });