Support editing arbitrary precisions for GlobeCoordinates #58
Conversation
| @@ -11,6 +11,7 @@ | |||
| "valueview-expert-unsupportedvalue-unsupporteddatatype": "Handling of values for \"$1\" data type is not yet supported.", | |||
| "valueview-expert-emptyvalue-empty": "empty", | |||
| "valueview-expert-globecoordinateinput-precision": "Precision:", | |||
| "valueview-expert-globecoordinateinput-customprecision": "custom value ($1)", | |||
There was a problem hiding this comment.
I would prefer something like "±$1° (custom)".
There was a problem hiding this comment.
The value is already formatted correctly. I don't care about the position of the value and »custom«, might just switch that. @lydiapintscher?
Fixes bug 65535.
| @@ -11,6 +11,7 @@ | |||
| "valueview-expert-unsupportedvalue-unsupporteddatatype": "Handling of values for \"$1\" data type is not yet supported.", | |||
| "valueview-expert-emptyvalue-empty": "empty", | |||
| "valueview-expert-globecoordinateinput-precision": "Precision:", | |||
| "valueview-expert-globecoordinateinput-customprecision": "custom ($1)", | |||
There was a problem hiding this comment.
After showing and chatting with @lydiapintscher we decided to use "special ($1)" (without any rounding) to not make users think this can be "customized" via the UI.
| if( value.custom ) { | ||
| this._customValue = this.rotator.options.values.push( value ) - 1; | ||
| this._$customItem = this.rotator._addMenuItem( value ); | ||
| value = value.value; |
There was a problem hiding this comment.
Uh, this is odd. What happens to the other values that are not custom? Are they supposed to be numbers?
There was a problem hiding this comment.
Numbers or strings, yes.
There was a problem hiding this comment.
Do you mean "number or object"? I may be wrong but it looks like you are introducing this. Why not always make it an object?
There was a problem hiding this comment.
Non-custom values are numbers or strings. It's like this:
Listrotators are initialized with an array of name/value pairs. These are the non-custom values which are always shown. Setting the rotator to one of these values is trivial using .value and ._setValue. On the other hand, our experts need to be able to set the Listrotator to a value which is not in this initial list (a ›custom‹ value). This is done by injecting a name/value pair into the Listrotator and then calling .value and ._setValue.
This module here (the ExpertExtender's Listrotator extension) wraps the ugly injection and handles cleaning up. In order to do that, it has to recognize a given value as either custom or not. It could also just try to find it in the Listrotator's value array, but that would be uglier, slower and more error-prone. So the expert has to explicitly say that a value it passes is not just a pre-configured value from the Listrotator's config, but indeed something new and custom. That's done via the custom property, and it also needs to pass a name and a value, because the Listrotator obviously needs a name and a value.
The only thing I can imagine we could change without rewriting a lot of Listrotator (which I tried to avoid with this patch) is changing the signature of the getUpstreamValue argument of ExpertExtender.Listrotator to always return an object, either { value: 'normal value' } or { value: 'custom value', label: 'label for custom value', custom: true }. I don't see much of an advantage doing that, though.
| this._customValue = null; | ||
| } | ||
| if( value.custom ) { | ||
| this._customValue = this.rotator.options.values.push( value ) - 1; |
There was a problem hiding this comment.
This variable name is misleading. Should be _customValueIndex or something like that.
It's also missing a declaration.
Support editing arbitrary precisions for GlobeCoordinates
Bug 65535.