From f6fbc8d9ffb6097b61982a8ec5cc4a06df8e6a96 Mon Sep 17 00:00:00 2001 From: shanyue Date: Fri, 20 Mar 2026 07:23:35 +0800 Subject: [PATCH] feat(svgcanvas): Trigger 'changed' event on text attribute modifications (#1085) * feat(svgcanvas): Trigger 'changed' event on text attribute modifications This update adds a call to the 'changed' event for various text attribute methods, ensuring that changes to text elements are properly tracked and reflected in the SVG canvas. The methods updated include setBold, setItalic, setTextAnchor, setLetterSpacing, setWordSpacing, setTextLength, setLengthAdjust, setFontFamily, setFontColor, and setFontSize. * refactor(svgcanvas): Simplify text attribute methods and optimize 'changed' event emission This update introduces helper functions to streamline the handling of selected text elements and their attribute changes. The methods setBold, setItalic, setTextAnchor, setLetterSpacing, setWordSpacing, and setTextLength now utilize these helpers to filter and notify only modified text elements, improving performance and clarity in event emissions. --- packages/svgcanvas/core/elem-get-set.js | 124 ++++++++++++++++-------- tests/unit/elem-get-set.test.js | 48 ++++++++- 2 files changed, 132 insertions(+), 40 deletions(-) diff --git a/packages/svgcanvas/core/elem-get-set.js b/packages/svgcanvas/core/elem-get-set.js index ad67464c..dd7bbdcb 100644 --- a/packages/svgcanvas/core/elem-get-set.js +++ b/packages/svgcanvas/core/elem-get-set.js @@ -655,14 +655,31 @@ const setStrokeAttrMethod = (attr, val) => { } } +const getSelectedTextElements = () => { + return svgCanvas.getSelectedElements().filter(el => el?.tagName === 'text') +} + +const getChangedTextElements = (textElements, attr, newValue) => { + const normalizedValue = String(newValue) + return textElements.filter((elem) => { + const oldValue = attr === '#text' ? elem.textContent : elem.getAttribute(attr) + return (oldValue || '') !== normalizedValue + }) +} + +const notifyTextChange = (textElements) => { + if (textElements.length > 0) { + svgCanvas.call('changed', textElements) + } +} + /** * Check if all selected text elements are in bold. * @function module:svgcanvas.SvgCanvas#getBold * @returns {boolean} `true` if all selected elements are bold, `false` otherwise. */ const getBoldMethod = () => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') + const textElements = getSelectedTextElements() return textElements.every(el => el.getAttribute('font-weight') === 'bold') } @@ -673,12 +690,16 @@ const getBoldMethod = () => { * @returns {void} */ const setBoldMethod = (b) => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') - svgCanvas.changeSelectedAttribute('font-weight', b ? 'bold' : 'normal', textElements) + const textElements = getSelectedTextElements() + const value = b ? 'bold' : 'normal' + const changedTextElements = getChangedTextElements(textElements, 'font-weight', value) + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('font-weight', value, changedTextElements) + } if (!textElements.some(el => el.textContent)) { svgCanvas.textActions.setCursor() } + notifyTextChange(changedTextElements) } /** @@ -686,8 +707,7 @@ const setBoldMethod = (b) => { * @returns {boolean} Indicates whether or not elements have the text decoration value */ const hasTextDecorationMethod = (value) => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') + const textElements = getSelectedTextElements() return textElements.every(el => (el.getAttribute('text-decoration') || '').includes(value)) } @@ -698,8 +718,7 @@ const hasTextDecorationMethod = (value) => { */ const addTextDecorationMethod = (value) => { const { ChangeElementCommand, BatchCommand } = svgCanvas.history - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') + const textElements = getSelectedTextElements() const batchCmd = new BatchCommand() textElements.forEach(elem => { @@ -726,8 +745,7 @@ const addTextDecorationMethod = (value) => { */ const removeTextDecorationMethod = (value) => { const { ChangeElementCommand, BatchCommand } = svgCanvas.history - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') + const textElements = getSelectedTextElements() const batchCmd = new BatchCommand() textElements.forEach(elem => { @@ -750,8 +768,7 @@ const removeTextDecorationMethod = (value) => { * @returns {boolean} `true` if all selected elements are in italics, `false` otherwise. */ const getItalicMethod = () => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') + const textElements = getSelectedTextElements() return textElements.every(el => el.getAttribute('font-style') === 'italic') } @@ -762,12 +779,16 @@ const getItalicMethod = () => { * @returns {void} */ const setItalicMethod = (i) => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') - svgCanvas.changeSelectedAttribute('font-style', i ? 'italic' : 'normal', textElements) + const textElements = getSelectedTextElements() + const value = i ? 'italic' : 'normal' + const changedTextElements = getChangedTextElements(textElements, 'font-style', value) + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('font-style', value, changedTextElements) + } if (!textElements.some(el => el.textContent)) { svgCanvas.textActions.setCursor() } + notifyTextChange(changedTextElements) } /** @@ -776,9 +797,12 @@ const setItalicMethod = (i) => { * @returns {void} */ const setTextAnchorMethod = (value) => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') - svgCanvas.changeSelectedAttribute('text-anchor', value, textElements) + const textElements = getSelectedTextElements() + const changedTextElements = getChangedTextElements(textElements, 'text-anchor', value) + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('text-anchor', value, changedTextElements) + } + notifyTextChange(changedTextElements) } /** @@ -787,12 +811,15 @@ const setTextAnchorMethod = (value) => { * @returns {void} */ const setLetterSpacingMethod = (value) => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') - svgCanvas.changeSelectedAttribute('letter-spacing', value, textElements) + const textElements = getSelectedTextElements() + const changedTextElements = getChangedTextElements(textElements, 'letter-spacing', value) + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('letter-spacing', value, changedTextElements) + } if (!textElements.some(el => el.textContent)) { svgCanvas.textActions.setCursor() } + notifyTextChange(changedTextElements) } /** @@ -801,12 +828,15 @@ const setLetterSpacingMethod = (value) => { * @returns {void} */ const setWordSpacingMethod = (value) => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') - svgCanvas.changeSelectedAttribute('word-spacing', value, textElements) + const textElements = getSelectedTextElements() + const changedTextElements = getChangedTextElements(textElements, 'word-spacing', value) + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('word-spacing', value, changedTextElements) + } if (!textElements.some(el => el.textContent)) { svgCanvas.textActions.setCursor() } + notifyTextChange(changedTextElements) } /** @@ -815,12 +845,15 @@ const setWordSpacingMethod = (value) => { * @returns {void} */ const setTextLengthMethod = (value) => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') - svgCanvas.changeSelectedAttribute('textLength', value, textElements) + const textElements = getSelectedTextElements() + const changedTextElements = getChangedTextElements(textElements, 'textLength', value) + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('textLength', value, changedTextElements) + } if (!textElements.some(el => el.textContent)) { svgCanvas.textActions.setCursor() } + notifyTextChange(changedTextElements) } /** @@ -829,12 +862,15 @@ const setTextLengthMethod = (value) => { * @returns {void} */ const setLengthAdjustMethod = (value) => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') - svgCanvas.changeSelectedAttribute('lengthAdjust', value, textElements) + const textElements = getSelectedTextElements() + const changedTextElements = getChangedTextElements(textElements, 'lengthAdjust', value) + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('lengthAdjust', value, changedTextElements) + } if (!textElements.some(el => el.textContent)) { svgCanvas.textActions.setCursor() } + notifyTextChange(changedTextElements) } /** @@ -852,13 +888,16 @@ const getFontFamilyMethod = () => { * @returns {void} */ const setFontFamilyMethod = (val) => { - const selectedElements = svgCanvas.getSelectedElements() - const textElements = selectedElements.filter(el => el?.tagName === 'text') + const textElements = getSelectedTextElements() + const changedTextElements = getChangedTextElements(textElements, 'font-family', val) svgCanvas.setCurText('font_family', val) - svgCanvas.changeSelectedAttribute('font-family', val, textElements) + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('font-family', val, changedTextElements) + } if (!textElements.some(el => el.textContent)) { svgCanvas.textActions.setCursor() } + notifyTextChange(changedTextElements) } /** @@ -868,8 +907,13 @@ const setFontFamilyMethod = (val) => { * @returns {void} */ const setFontColorMethod = (val) => { + const textElements = getSelectedTextElements() + const changedTextElements = getChangedTextElements(textElements, 'fill', val) svgCanvas.setCurText('fill', val) - svgCanvas.changeSelectedAttribute('fill', val) + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('fill', val, changedTextElements) + } + notifyTextChange(changedTextElements) } /** @@ -895,12 +939,16 @@ const getFontSizeMethod = () => { * @returns {void} */ const setFontSizeMethod = (val) => { - const selectedElements = svgCanvas.getSelectedElements() + const textElements = getSelectedTextElements() + const changedTextElements = getChangedTextElements(textElements, 'font-size', val) svgCanvas.setCurText('font_size', val) - svgCanvas.changeSelectedAttribute('font-size', val) - if (!selectedElements[0]?.textContent) { + if (changedTextElements.length > 0) { + svgCanvas.changeSelectedAttribute('font-size', val, changedTextElements) + } + if (!textElements.some(el => el.textContent)) { svgCanvas.textActions.setCursor() } + notifyTextChange(changedTextElements) } /** diff --git a/tests/unit/elem-get-set.test.js b/tests/unit/elem-get-set.test.js index faf8cd6d..0f0b263a 100644 --- a/tests/unit/elem-get-set.test.js +++ b/tests/unit/elem-get-set.test.js @@ -1,4 +1,4 @@ -import { beforeEach, afterEach, describe, expect, it } from 'vitest' +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest' import { NS } from '../../packages/svgcanvas/core/namespaces.js' import * as history from '../../packages/svgcanvas/core/history.js' import dataStorage from '../../packages/svgcanvas/core/dataStorage.js' @@ -35,7 +35,8 @@ describe('elem-get-set', () => { clear () {} }, runExtensions () {}, - call () {}, + call: vi.fn(), + changeSelectedAttribute: vi.fn(), getDOMDocument () { return document }, getSvgContent () { return svgContent }, getSelectedElements () { return this.selectedElements || [] }, @@ -51,6 +52,10 @@ describe('elem-get-set', () => { }, addCommandToHistory (cmd) { historyStack.push(cmd) + }, + setCurText () {}, + textActions: { + setCursor () {} } } svgContent.setAttribute('width', '100') @@ -232,4 +237,43 @@ describe('elem-get-set', () => { expect(localCanvas.contentW).toBe(200) expect(localCanvas.contentH).toBe(150) }) + + it('setBold() emits changed only for modified text elements', () => { + const text = createSvgElement('text') + text.textContent = 'Hello' + const rect = createSvgElement('rect') + svgContent.append(text, rect) + canvas.selectedElements = [text, rect] + + canvas.setBold(true) + + expect(canvas.changeSelectedAttribute).toHaveBeenCalledWith('font-weight', 'bold', [text]) + expect(canvas.call).toHaveBeenCalledWith('changed', [text]) + }) + + it('setBold() skips changed event for no-op text updates', () => { + const text = createSvgElement('text') + text.textContent = 'Hello' + text.setAttribute('font-weight', 'bold') + svgContent.append(text) + canvas.selectedElements = [text] + + canvas.setBold(true) + + expect(canvas.changeSelectedAttribute).not.toHaveBeenCalled() + expect(canvas.call).not.toHaveBeenCalled() + }) + + it('setFontColor() ignores non-text selections when emitting changed', () => { + const text = createSvgElement('text') + text.textContent = 'Hello' + const rect = createSvgElement('rect') + svgContent.append(text, rect) + canvas.selectedElements = [text, rect] + + canvas.setFontColor('#f00') + + expect(canvas.changeSelectedAttribute).toHaveBeenCalledWith('fill', '#f00', [text]) + expect(canvas.call).toHaveBeenCalledWith('changed', [text]) + }) })