[WIP] Improved barcode options #171
Conversation
| @@ -4,6 +4,10 @@ class Barcode{ | |||
| this.text = options.text || data; | |||
| this.options = options; | |||
| } | |||
|
|
|||
| static options() { | |||
There was a problem hiding this comment.
Did you think to make it as a property, but not as a function?
There was a problem hiding this comment.
I thought about it. The main reason why I wanted to make this a function is that it will make inheritance of options easier. If for all CODE128 codes have one option but we want to add additional options to that sub-symbology it can easily be done like follow.
static options() {
return Object.assign({}, super.options(), {
somenewoptions: {}
});
}| @@ -25,7 +25,9 @@ class ErrorHandler{ | |||
| wrapBarcodeCall(func){ | |||
| try{ | |||
| var result = func(...arguments); | |||
| this.api._options.valid(true); | |||
| if (typeof this.api._options.valid !== 'undefined') { | |||
There was a problem hiding this comment.
this.api._options.valid !== undefined should be enough, I think.
There was a problem hiding this comment.
There was a problem hiding this comment.
Oh, you can do it in old specs as I see (http://www.ecma-international.org/ecma-262/5.1/#sec-15.1.1.3). But for a public library your way is better :)
|
|
||
| // Convert an option to a specified type | ||
| function convertOption(data, type) { | ||
| if (type === 'string') { |
There was a problem hiding this comment.
Maybe check type before:
if (typeof data !== type) {
// do conversions
}
return data;There was a problem hiding this comment.
type is the type it should be converted TO, not the type it already is. Or I might have misunderstood this.
There was a problem hiding this comment.
For example convertOption(10, 'number') causes return parseInt(data, 10); anyway. I suggest check type of data before and avoid any unnecessary conversion. Or I missed something :)
|
And please, do not forget about options for CODE128 :) |
|
Of course I will not 😉 I just added EAN to test with and there is a lot of other options thats need to be added. |
This is a WIP of how I want to implement barcode specific options. It does also allow for the awful
optionsFromStrings.jsto be removed.It is not done and does not yet work fully, but what do you think of the general idea @SanichKotikov ?