Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(TS): Text SVG export mixin #8486

Merged
merged 19 commits into from
Dec 31, 2022
Merged

chore(TS): Text SVG export mixin #8486

merged 19 commits into from
Dec 31, 2022

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Dec 4, 2022

Motivation

Description

Last file accept canvas files

Changes

  • src/mixins/itext.svg_export.ts:
    The diff is very harsh... Perhaps better to view commit by commit? Since the first commit is the transfomer's work and the second is just generic migration and moving a method or 2.
    Renamed to src/mixins/text.svg_export.ts since Github doesn't pick it up because the diff is too large. Revert at will, ebbbffd
  • src/mixins/object.svg_export.ts: cleanup
  • src/shapes/itext.class.ts: jsdoc
  • src/util/misc/svgParsing.ts: extract utils to here

Gist

In Action

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

Build Stats

file / KB (diff) bundled minified
fabric 948.201 (-2.985) 305.577 (-0.454)

This reverts commit cb7404f.

@asturur revert this commit to rename the file once you are done reviewing (done for diff readability)
@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

Coverage after merging ts-itext-svg into master will be

82.90%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
HEADER.js54%48.15%0%63.64%12, 14, 14, 14, 14, 14, 16–17, 21, 21–22, 22, 22, 24, 24, 26, 28, 30–31, 77, 77, 77
src
   cache.ts97.06%90%100%100%56
   config.ts77.27%66.67%66.67%84.62%130, 138–140, 151–153
   constants.ts100%100%100%100%
   intersection.class.ts100%100%100%100%
   pattern.class.ts92.31%85.71%100%96.43%120, 126, 137, 146, 98
   point.class.ts100%100%100%100%
   shadow.class.ts98.51%96%100%100%157
src/brushes
   base_brush.class.ts100%100%100%100%
   circle_brush.class.ts1.52%0%0%2%101, 103–105, 114, 114, 114, 116, 118, 120–122, 124–127, 135, 142, 144, 24, 29–30, 38–42, 46–50, 57–60, 68–72, 74, 82, 82, 82, 82, 82–83, 85, 85, 85–88, 90, 98–99
   pattern_brush.class.ts97.14%87.50%100%100%22
   pencil_brush.class.ts91.91%85.42%100%93.64%123–124, 153, 153–155, 277, 281, 286–287, 69–70, 85–86
   spray_brush.class.ts1.16%0%0%1.56%100–102, 104–105, 113, 113, 113, 113, 113–114, 116–117, 124–125, 127, 129–133, 142, 146–147, 147, 155, 155, 155–158, 160–163, 167–168, 170, 172–175, 178, 185–186, 188, 190–191, 193, 200–201, 203–204, 207, 207, 214, 214, 218, 23–24, 26–28, 28, 28–30, 34, 43, 50, 57, 64, 71, 78, 90–92
src/canvas
   canvas.class.ts92.52%88.15%94.12%95.45%1150, 1150–1151, 1154, 1174, 1174, 1236, 1272–1273, 1292–1293, 1301–1302, 1323, 1331, 1444–1445, 1447–1448, 1468–1469, 1632–1633, 1637, 509, 576–577, 582, 592, 721–722, 724–725, 725, 725, 771–772, 833–834, 837, 839, 884–886, 916, 921–922, 951–952
   canvas_events.ts78.10%75%82.81%79.52%1003–1004, 1004, 1004–1006, 1008–1009, 1009, 1009, 1011, 1019, 1019, 1019–1021, 1021, 1021, 1027–1028, 1036–1037, 1037, 1037–1038, 1043, 1045, 1075–1077, 1080–1081, 1085–1086, 1184, 1204–1206, 1209–1210, 1282, 1369, 1402, 145, 1496–1497, 1503, 1507–1508, 1524, 1546, 1593, 1598, 1637, 170, 318–319, 319, 319–320, 320, 323–327, 332, 334, 346–348, 351, 368, 368, 368–369, 369, 369–370, 378, 383–384, 384, 384–385, 387, 396, 402–403, 403, 403, 439, 443, 443, 443, 443, 443–444, 446, 521, 523, 523, 523–525, 545, 547, 547, 547–548, 548, 548, 551, 551, 551–552, 555, 564–565, 567–568, 570, 570–571, 573–574, 586–587, 587, 587–588, 590–594, 600, 607, 647, 647, 647, 649, 651–655, 661, 667, 667, 667–668, 670, 673, 678, 691, 718, 774–775, 775, 775–776, 778, 781–782, 782, 782–783, 785–786, 789, 789–791, 794–795, 805, 887, 901, 908, 929, 961, 982–983, 989
   canvas_gestures.mixin.ts9.21%5.56%7.14%11.36%101, 114, 127, 139, 148, 157–161, 170, 172, 172, 172–173, 175–176, 189, 198, 207, 211, 30, 30, 30–31, 31, 31, 31, 36, 39–40, 40, 40–41, 47, 50, 58, 58, 58, 58, 58–59, 62–64, 66–67, 69, 71, 71, 71–73, 76, 78, 88
   static_canvas.class.ts94.57%88.89%97.92%97.13%1130–1131, 1131, 1131–1132, 1252, 1262, 1316–1317, 1320, 1355–1356, 1434, 1443, 1448, 1497–1498, 1726, 1726–1727, 1777, 1780, 1783, 1783, 1783, 1786, 1789, 1789, 1789, 309, 345, 358, 414–415, 417–418, 427, 431–432, 434–435, 890
src/color
   color.class.ts92.16%86.49%100%94.29%330–331, 335–336, 339–340, 58, 88–89, 89, 91, 91, 91–92, 94–95
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   index.ts100%100%100%100%
   util.ts100%100%100%100%
src/controls
   actions.ts100%100%100%100%
   changeWidth.ts100%100%100%100%
   control.class.ts93.98%88.89%90.91%97.78%236, 320, 320, 355
   controls.render.ts81.63%78%100%84.78%106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
   default_controls.ts86.67%66.67%100%100%125, 132
   drag.ts100%100%100%100%
   rotate.ts20%12.50%50%22.22%45, 51, 51, 51–52, 55–57, 59, 59, 59, 59, 59–61, 61, 61–63, 65, 65, 65–67, 67, 67–68, 73, 73, 73–74, 76, 78, 80–81
   scale.ts93.41%92.68%100%93.59%129–130, 132–134, 148–149, 181–183, 42
   scaleSkew.ts81.25%64.29%100%92.31%27, 29, 29, 29, 31, 33
   skew.ts91.03%79.31%100%97.67%130–131

reverts 3bed0ab

@asturur revert this if you want. I thought the diff would be readable without it but it is unreadable regardless
Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

READY

renamed src/mixins/itext.svg_export.ts to src/mixins/text.svg_export.ts
Github doesn't pick it up because the diff is too large. Revert at will, ebbbffd

@@ -170,25 +177,6 @@ export class FabricObjectSVGExportMixin {
return `${svgTransform}${additionalTransform}" `;
}

_setSVGBg(textBgRects: string[]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used by text, refactored into text svg export mixin

* @param value
* @returns
*/
export function getSvgColorString(prop: string, value?: any) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now used by Text instead of _getFillAttributes

index.js Outdated
@@ -55,8 +55,8 @@ import './src/filters/gamma_filter.class'; // optional image_filters
import './src/filters/composed_filter.class'; // optional image_filters
import './src/filters/hue_rotation.class'; // optional image_filters
import './src/shapes/text.class'; // optional text
import './src/mixins/itext.svg_export'; // optional svg
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming is wrong, it is a Text mixin. So it should be applied before subclassing IText

textAndBg = this._getSVGTextAndBg(offsets.textTop, offsets.textLeft);
return this._wrapSVGTextAndBg(textAndBg);
},
function createSVGRect(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formally _setSVGBg and _pushTextBgRect


/* _TO_SVG_START_ */

function getSvgColorString(prop: string, value?: any) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extracted to util and renamed to colorPropToSVG

@ShaMan123 ShaMan123 requested a review from asturur December 4, 2022 10:58
/**
* @deprecated unused
*/
_getSVGLineTopOffset(lineIndex: number) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method is dead
can we remove it?

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

READY

@ShaMan123 ShaMan123 closed this Dec 5, 2022
@ShaMan123 ShaMan123 reopened this Dec 5, 2022
@asturur asturur merged commit 4e83d3d into master Dec 31, 2022
frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023
@ShaMan123 ShaMan123 deleted the ts-itext-svg branch February 6, 2023 07:05
ShaMan123 added a commit that referenced this pull request Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants