Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Style attribute enumerations redundantly include layer class names #6577

Closed
1ec5 opened this issue Oct 4, 2016 · 6 comments · Fixed by #6589
Closed

Style attribute enumerations redundantly include layer class names #6577

1ec5 opened this issue Oct 4, 2016 · 6 comments · Fixed by #6589
Assignees
Labels
iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor runtime styling
Milestone

Comments

@1ec5
Copy link
Contributor

1ec5 commented Oct 4, 2016

The style layer classes’ paint attributes are named somewhat redundantly, for instance -[MGLCircleStyleLayer circleOpacity]. Since there’s no possibility that, say, lineOpacity would ever exist on the same class, we should remove the first word from this property and any property like it. It’s totally fine if, say, width means something slightly different on circle and line style layers, since the meaning on each is consistent with developer expectations.

Putting on the v3.4.0 milestone because we won’t be able to change this later without breaking backwards compatibility.

/cc @frederoni @incanus

@1ec5 1ec5 added iOS Mapbox Maps SDK for iOS refactor macOS Mapbox Maps SDK for macOS runtime styling labels Oct 4, 2016
@1ec5 1ec5 added this to the ios-v3.4.0 milestone Oct 4, 2016
@1ec5
Copy link
Contributor Author

1ec5 commented Oct 4, 2016

This would also simplify enum names. For example, MGLLineStyleLayerLineTranslateAnchorMap would become MGLLineStyleLayerTranslateAnchorMap. We could go a little further, shortening it to MGLLineTranslateAnchorMap.

@incanus
Copy link
Contributor

incanus commented Oct 4, 2016

Since there’s no possibility that, say, lineOpacity would ever exist on the same class

Is this true? For fills, we've long talked about allowing for variable-width outline (mapbox/mapbox-gl-style-spec#223, #1737) and I could see possibly getting a circle outline someday.

@1ec5
Copy link
Contributor Author

1ec5 commented Oct 4, 2016

If so, I think the style specification would call the property something else, like line-stroke-opacity, so there would be no collision. As it is, the only properties that aren’t prefixed pro forma with the layer type are the layout properties that are shared among the types.

@1ec5
Copy link
Contributor Author

1ec5 commented Oct 4, 2016

@lucaswoj points out that many symbol properties aren’t prefixed with symbol- but are instead prefixed with text- or icon-. Could we use a heuristic like:

  • If the first part of the property name matches the layer type, omit it.
  • Otherwise, use the property name verbatim.

That would allow us to differentiate where needed but avoid redundancy in most of the API. However, it would assume that the circle layer type can’t have a line-color property; it’d have to be called circle-line-color instead. I think that’s a reasonable constraint to put on the style specification.

Even if we can’t shorten the property names, I still think it’d be desirable to shorten the unwieldy enum values.

@jfirebaugh
Copy link
Contributor

I suggest you just accept the redundancy in property names. That's the most predictable and future-proof solution.

Shortening the enum values seems fine. Those aren't prefixed in the style spec to begin with.

@1ec5 1ec5 changed the title Style attribute names redundantly include layer type Style attribute enumerations redundantly include layer class names Oct 4, 2016
@1ec5
Copy link
Contributor Author

1ec5 commented Oct 4, 2016

Alright, not the end of the world. Retitled to reflect the focus on enumeration names and values instead.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS refactor runtime styling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants