-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
shorten SVG inline syntax #445
base: master
Are you sure you want to change the base?
Conversation
4dd45b1
to
8841aa4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. A few comments added.
There were also (needed) code style changes, but please never make them in a PR doing other things, it makes it a lot harder to see what is actualy going on with the PR.
src/renderers/svg.js
Outdated
@@ -15,13 +15,21 @@ class SVGRenderer{ | |||
var currentX = this.options.marginLeft; | |||
|
|||
this.prepareSVG(); | |||
|
|||
// Create Background | |||
if(!this.options.background.match(/transparent|#[a-f\d]{3}(?:[a-f\d]{3})?0/i)){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not rga/hsl style colors etc. Why is the regexp added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i focused on the JsBarcode
default values and common shortcut & will slow down the parsing
[rgb(a),hsl(a),(ok)lch,lab,hwb]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
legacyColorRegex = /(?:rgb|hsl)a?\([^,]+?\s*,\s*[^,]+?\s*,\s*[^,]+?\s*,\s*0(?:[\.0%\s]*)?\)/
modernColorRegex = /(ok)?[a-z]{3}\([^\/]+\/\s*0(?:[\.0%\s]*)?\)/
src/renderers/svg.js
Outdated
for(let i = 0; i < this.encodings.length; i++){ | ||
var encoding = this.encodings[i]; | ||
var encodingOptions = merge(this.options, encoding.options); | ||
|
||
var group = this.createGroup(currentX, encodingOptions.marginTop, this.svg); | ||
|
||
this.setGroupOptions(group, encodingOptions); | ||
if(!encodingOptions.lineColor.match(/black|#000(?:000)?f?/i)){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment that it will default to black and that it will shorten the SVG syntax.
697fcf5
to
1db63b9
Compare
[FIX] switch unicode to ascii parsing |
may consider an
option
to deactivate width/height @ "header" for easier SVG handelingJsBarcode/src/renderers/svg.js
Lines 130 to 131 in 5ed2a2b