Skip to content

fix(plugin:export): it should have default value 'true' through doc #4111

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 24, 2024
Merged

fix(plugin:export): it should have default value 'true' through doc #4111

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bf5d472
fix(plugin:export): it should have default value 'true' through doc
Sorryhx Dec 20, 2023
4b04fa0
fix(plugin:export): more logical condition after last fix
Sorryhx Dec 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion packages/x6-plugin-export/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,16 @@ export class Export extends Basecoat<Export.EventArgs> implements Graph.Plugin {
toSVG(callback: Export.ToSVGCallback, options: Export.ToSVGOptions = {}) {
this.notify('before:export', options)

// to keep pace with it's doc description witch, the default value should be true.
// without Object.hasOwn method cause by ts config target.
// without instance.hasOwnProperty method cause by ts rule.
// the condition will be false if these properties have been set undefined in the target,
// but will be true if these properties are not in the target, cause the doc.
!Object.prototype.hasOwnProperty.call(options, 'copyStyle') &&
Copy link
Contributor

@NewByVector NewByVector Jan 9, 2024

Choose a reason for hiding this comment

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

Because the following judgment is options.copyStyles !== false, so if it is not passed, it is also executed in accordance with true.

Copy link
Contributor Author

@Sorryhx Sorryhx Jan 9, 2024

Choose a reason for hiding this comment

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

In fact, we just have a difference about what is the true . It will execute correctly under the strict-Boolean-type. The options.copyStyles !== false is a strict un-equality which likes ===.But, the serializeImages condition in line 209 is not strict which likes ==.So, In my option, we should keep the same style about the serializeImages and copyStyle condition what are both the Boolean value through the docs on the one hand, on the other hand, It should excute false statement when the value is either '', or ``, or null,or undefined,or NAN. All above these just beacuse JS is a fuck weak type language😓.

Copy link
Contributor

@NewByVector NewByVector Jan 24, 2024

Choose a reason for hiding this comment

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

👍

(options.copyStyles = true)
!Object.prototype.hasOwnProperty.call(options, 'serializeImages') &&
(options.serializeImages = true)

const rawSVG = this.view.svg
const vSVG = Vector.create(rawSVG).clone()
let clonedSVG = vSVG.node as SVGSVGElement
Expand Down Expand Up @@ -94,7 +104,7 @@ export class Export extends Basecoat<Export.EventArgs> implements Graph.Plugin {
// custom stylesheets onto the `style` attribute of each of the nodes
// in SVG.

if (options.copyStyles !== false) {
if (options.copyStyles) {
const document = rawSVG.ownerDocument!
const raws = Array.from(rawSVG.querySelectorAll('*'))
const clones = Array.from(clonedSVG.querySelectorAll('*'))
Expand Down