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

fix(gatsby-remark-images): allow tracedSVG to accept object with settings #28242

Merged
merged 2 commits into from
Dec 9, 2020

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Nov 23, 2020

Description

Currently plugin validation schema only supports true/false, but plugin does support setting detailed settings with object notation too -

let args = typeof options.tracedSVG === `object` ? options.tracedSVG : {}

Related Issues

Fixes #28217

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Nov 23, 2020
Comment on lines +63 to +76
// this plugin also allow to use key names and not exact values
`TURNPOLICY_BLACK`,
`TURNPOLICY_WHITE`,
`TURNPOLICY_LEFT`,
`TURNPOLICY_RIGHT`,
`TURNPOLICY_MINORITY`,
`TURNPOLICY_MAJORITY`,
// it also allow using actual policy values
Potrace.TURNPOLICY_BLACK,
Potrace.TURNPOLICY_WHITE,
Potrace.TURNPOLICY_LEFT,
Potrace.TURNPOLICY_RIGHT,
Potrace.TURNPOLICY_MINORITY,
Potrace.TURNPOLICY_MAJORITY
Copy link
Contributor Author

Choose a reason for hiding this comment

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

2 ways of setting it is covered by

// Translate Potrace constants (e.g. TURNPOLICY_LEFT, COLOR_AUTO) to the values Potrace expects
const { Potrace } = require(`potrace`)
const argsKeys = Object.keys(args)
args = argsKeys.reduce((result, key) => {
const value = args[key]
result[key] = Potrace.hasOwnProperty(value) ? Potrace[value] : value
return result
}, {})

color: Joi.string().default(`lightgray`),
background: Joi.string().default(`transparent`),
})
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Defaults and some of extra restrictions are coming from:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also - maybe it's safer to remove defaults, because it's not this plugin that set them. My main motivation to add those defaults is that maybe they are (or will be) displayed in some form - if they are not, then better remove them as they can get out of sync

@pieh pieh added topic: media Related to gatsby-plugin-image, or general image/media processing topics type: bug An issue or pull request relating to a bug in Gatsby and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Nov 23, 2020
Copy link
Contributor

@mfrachet mfrachet left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me :). What do you think of that @mxstbr ?

@pieh pieh merged commit 23ecf2d into master Dec 9, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix/gatsby-remark-images/tracedSVG-schema branch December 9, 2020 09:46
pieh added a commit that referenced this pull request Dec 9, 2020
…ings (#28242)

* fix(gatsby-remark-images): allow tracedSVG to accept object with settings

* fix test setup that was testing same thing twice

(cherry picked from commit 23ecf2d)
pieh added a commit that referenced this pull request Dec 9, 2020
…ings (#28242) (#28552)

* fix(gatsby-remark-images): allow tracedSVG to accept object with settings

* fix test setup that was testing same thing twice

(cherry picked from commit 23ecf2d)

Co-authored-by: Michal Piechowiak <[email protected]>
@pieh
Copy link
Contributor Author

pieh commented Dec 10, 2020

Published in [email protected]

pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
…ings (gatsbyjs#28242)

* fix(gatsby-remark-images): allow tracedSVG to accept object with settings

* fix test setup that was testing same thing twice
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: media Related to gatsby-plugin-image, or general image/media processing topics type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin options error for gatsby-remark-images
2 participants