Conversation
|
I restored the original logic of serialization to avoid text fixtures changes. Cna you rerun it and update the files |
- Set NumberFormatter decimalSeparator to "." for consistent formatting - Update test fixtures to reflect period-separated decimal values - Ensure transform matrix serialization works regardless of system locale
| createReference(name: "paint-color-03-t", version: v12) | ||
| createReference(name: "paint-color-201-t", version: v12) | ||
| createReference(name: "paint-fill-04-t", version: v12) | ||
| createReference(name: "paint-fill-06-t", version: v12) |
There was a problem hiding this comment.
The test is present in the orginally repo. We need this line to regenerate the ref file.
| import Combine | ||
| #endif | ||
|
|
||
| public class SVGDefs: SVGGroup {} |
There was a problem hiding this comment.
You are not parsing the defs? why have it then?
There was a problem hiding this comment.
<defs> is parsed. Currently, it is parsed like <g>
| self = .angle(0) | ||
| return | ||
| } | ||
| if rawValue == "auto" { |
Source/Model/Nodes/SVGMarker.swift
Outdated
| import Combine | ||
| #endif | ||
|
|
||
| public class SVGMarker: SVGNode { |
There was a problem hiding this comment.
This is probably need to be a subclass of SVGGroup
There was a problem hiding this comment.
It is intentional because the constructor of SVGGroup (public init(contents: [SVGNode], transform: CGAffineTransform = .identity, opaque: Bool = true, opacity: Double = 1, clip: SVGUserSpaceNode? = nil, mask: SVGNode? = nil)) is very different from SVGMarker.
There was a problem hiding this comment.
I checked the SVG spec and most props allowed on SVGGroup are allowed on Marker (transform, clip, mask ...) so it makes sense for SVGMarker to be a SVGGroup imo.
Here is a patch you could apply to update the init
0001-Refactor-SVGMarker-inheritance-and-remove-code-dupli.patch
| case percent(CGFloat) | ||
| case pixels(CGFloat) | ||
|
|
||
| static let zero: SVGLength = .pixels(0) |
There was a problem hiding this comment.
I'd just remove this, the naming for tthis is unclear. .pixels(0) != .percent(0)
Co-authored-by: khoi <6994441+khoi@users.noreply.github.com>
2 participants
https://goodnotes.atlassian.net/browse/FFA-407
Tested in XCode
Reviewers should check whether the changes in the test fixtures are acceptable.