Closed nvkelso closed 7 years ago
/cc @tallytalwar
What I know from writing the ES code, this is not a bug and is exactly how the property merging behaves on an import. Also having an empty value (null value) is a valid yaml.
The question though remains is how to communicate the use of an empty style property in the base icon styles. Also explain/communicate how ordering of imports affect the usage of this empty style property (mix
in our case here)?
Can you guys link to the scene YAML in question, for context?
On Mon, Sep 25, 2017 at 4:22 PM, Varun notifications@github.com wrote:
What I know from writing the ES code, this is not a bug and is exactly how the property merging behaves on an import. Also having an empty value (null value) is a valid yaml.
The question though remains is how to communicate the use of an empty style property in the base icon styles. Also explain how ordering of imports affect the usage of this empty style property (mix in our case here)?
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/tangrams/refill-style/issues/74#issuecomment-332000595, or mute the thread https://github.com/notifications/unsubscribe-auth/AABBXTS2LTaKOP1j0rhRbVlz4R8otAijks5smAtpgaJpZM4PjLMk .
So yes, this works as far as currently implemented merging behavior is intended, however I'm concerned that it's a pretty brittle pattern (past experience suggests it likely breaks in future releases of the styles). In particular:
mix:
to unset the icon library. (Besides the coupling, it's also rather obscure, even with the comment.)In an ideal solution, turning icon colorization on/off would be decoupled both from import order and between styles -- something like a global.mapzen_icon_library_colorize
flag that could default false
and the Refill style could enable, with other users enabling/disabling as desired.
I haven't been able to spend enough time to try rewriting this in that way, but I wonder if something could be done with globals
that either redefine the mapzen_icon_library
(so that rather than merging, they are kept as two separate definitions that are simply "switched between") or colorized-icons
style entirely (perhaps with an empty style defined as {}
?).
@matteblair I'd like your thoughts here on the general issue and speculation of other solutions.
Another proposed solution is to expose a global to control the mixing style on mapzen-icon-library
. It would have been a bit more simpler using a boolean global, had we supported JS functions on mix
. I do not think these are supported though.
Example:
Specify the mix style using a global in the refill color themes.
control the mixing of color by specifying a global in the final scene:
# when following is defined, it overrides the mix style on the imported color theme, otherwise the icons gets the colored theme from the imported refill theme.
global:
sdk-icon-color-style:
import:
- https://mapzen.com/carto/refill-style/9/refill-style.zip
- https://raw.githubusercontent.com/tangrams/refill-style/varun/test-colorized-global/themes/color-purple-green.yaml
- https://mapzen.com/carto/refill-style/9/themes/label-10.zip
- https://raw.githubusercontent.com/tangrams/tron-style/varun/test-colorized-global/themes/tron-icons.yaml
Here order of import won't determine the coloring or non-coloring of the base style icons. But the behavior will be explicitly defined by the use of the global.
... I wonder if something could be done with globals that either redefine the mapzen_icon_library (so that rather than merging, they are kept as two separate definitions that are simply "switched between") or colorized-icons style entirely (perhaps with an empty style defined as {}?).
I'd like for the Mapzen Icon Library work to have a consistent import and usage regardless of basemap style – remember these 6 different {style}-icon.zip
files are meant to be used both in our Mapzen basemap styles and independent off them in customer's own projects. Keeping the Tangram style
and texture
names the same enables the Cartography documentation to be straight forward.
In an ideal solution, turning icon colorization on/off would be decoupled both from import order and between styles -- something like a global.mapzen_icon_library_colorize flag that could default false and the Refill style could enable, with other users enabling/disabling as desired.
If I understand right:
refill-style.yaml
global.mapzen_icon_library_colorize: true
refill-icons.yaml
global.colorize_icons
could be evaluated either:
mix:
https://github.com/tangrams/refill-style/blob/1ed9bfd71151aaec2b1290d9b2450928a62bfb3b/themes/refill-icons.yaml#L6
Are Javascript functions supported on mix:
now by both Tangram JS and ES?refill-icons.yaml
used with refill-style.yaml
then the icons could be colorized with any included color-{theme}.yaml
, or on your ownwalkabout-icons.yaml
import: [ refill-style.yaml, walkabout-icons.yaml]
as refill-style.yaml
already imports refill-icons.yaml
implicitly so the mix / uniforms would already be set unless unset later by walkabout-icons.yaml
.style
definition into each color theme (and remove it from the default color-black.yaml
used in refill-style.yaml
, but that seems like much hoop jumping and duplicating of code.global.mapzen_icon_library_colorize: true
in addition to doing the two imports.walkabout-icons.yaml
unsetting colorized-icons: {}
(versus the current mix:
which unreferenced colorized-icons
from the mapzen_icon_library
style) if that does work and produces the same output.
Personally I'm fine with the order of imports mattering – that's already true for some of our other work.
To clarify:
theme/color-{name}.yaml
imports for Refill to colorize the basemap colors & the icons...theme/color-{name}.yaml
imports to only colorize the basemap colors and not the icons.theme/color-{name}.yaml
should also colorize the Walkabout & etc icons. But that's just less useful.I'm looking at this now, trying to think of a simpler or more robust solution. Will let you know by the end of the day.
I think the solution @tallytalwar proposed is rather good. As I understand it, it boils down to icon colorization being toggled by the final value of a global. In the names used above:
global: { sdk-icon-color-style: null }
turns off icon colorization.
global: { sdk-icon-color-style: colorized-icons }
turns on icon colorization.
This isn't as ideal as a simple boolean flag, but is far more descriptive than the order of various imports. It also enables all of the behaviors that @nvkelso described above:
theme/color-{name}.yaml
files can define global: { sdk-icon-color-style: colorized-icons }
to enable icon colorization by default.global: { sdk-icon-color-style: null }
to enable only basemap colors and not icon colorization.Cool, I'll prototype the recommendation today with a new beta release. Thank you!
Here we add a
mix: colorized_icons
, and then later we allow import ofmix:
in other Mapzen basemap styles to unset that. This works, but it's not clear if it's a bug. Discuss!Refill:
https://github.com/tangrams/refill-style/blob/1ed9bfd71151aaec2b1290d9b2450928a62bfb3b/themes/refill-icons.yaml#L6
Tron:
https://github.com/tangrams/tron-style/blob/828f0d3b1444daff61da19b440d2074d9d310d8c/themes/tron-icons.yaml#L4-L6
Import with Tron icons (full color):
Import with Tron icons (as colorized ala Refill), by moving the import order: