Open
Conversation
xerc
force-pushed
the
patch-1
branch
3 times, most recently
from
4dd45b1 to
8841aa4
Compare
lindell
reviewed
Feb 29, 2024
Owner
lindell
left a comment
lindell
left a comment
There was a problem hiding this comment.
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
| this.prepareSVG(); | ||
|
|
||
| // Create Background | ||
| if(!this.options.background.match(/transparent|#[a-f\d]{3}(?:[a-f\d]{3})?0/i)){ |
Owner
There was a problem hiding this comment.
This does not rga/hsl style colors etc. Why is the regexp added?
Author
There was a problem hiding this comment.
i focused on the JsBarcode default values and common shortcut & will slow down the parsing
[rgb(a),hsl(a),(ok)lch,lab,hwb]
Author
There was a problem hiding this comment.
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
| var group = this.createGroup(currentX, encodingOptions.marginTop, this.svg); | ||
|
|
||
| this.setGroupOptions(group, encodingOptions); | ||
| if(!encodingOptions.lineColor.match(/black|#000(?:000)?f?/i)){ |
Owner
There was a problem hiding this comment.
Please add a comment that it will default to black and that it will shorten the SVG syntax.
xerc
force-pushed
the
patch-1
branch
5 times, most recently
from
697fcf5 to
1db63b9
Compare
Author
|
[FIX] switch unicode to ascii parsing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
may consider an
optionto deactivate width/height @ "header" for easier SVG handelingJsBarcode/src/renderers/svg.js
Lines 130 to 131 in 5ed2a2b