-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Rename textField, textFont, textSize, circlePitchScale, *Translate, *TranslateAnchor #7603
Conversation
@1ec5, thanks for your PR! By analyzing this pull request, we identified @boundsj, @friedbunny and @incanus to be potential reviewers. |
0244d2e
to
3043453
Compare
On @jfirebaugh’s suggestion in #6098 (comment), I also replaced Finally – and I mean finally – |
@@ -153,20 +153,32 @@ typedef NS_ENUM(NSUInteger, MGLCircleTranslateAnchor) { | |||
`NSValue` object containing a `CGVector` struct set to 0 points from the left | |||
and 0 points from the top. Set this property to `nil` to reset it to the | |||
default value. | |||
|
|||
This attribute corresponds to the <a | |||
href="https://www.mapbox.com/mapbox-gl-style-spec/#layout-circle-circle-translate"><code>circle-translate</code></a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This URL leads to the root of the style spec. Isn't the correct ref #paint-circle-translate
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch – all the paint property links were broken. Fixed in d3a2ffe.
…tch-scale Reduced the likelihood that the developer might attempt to set textField to an NSTextField or UITextField, or textFont to an NSFont or UIFont, or textSize to a CGSize or NSSize, or circlePitchScale to a number (given iconScale).
3043453
to
bf8d037
Compare
Renamed more properties according to mapbox/mapbox-gl-native#7603.
This PR renames the following properties:
MGLSymbolStyleLayer.textField
totext
, in anticipation of Rename text-field to text mapbox-gl-style-spec#646The developer would expect a
textField
property to hold an NSTextField or UITextField, or at least some kind of editable control, but not an NSString. There is such a thing as NSText on macOS, but no one ever uses it directly. On the other hand,UILabel.text
, analogous to thetext-field
layout property, is one of the most commonly used properties on iOS.MGLSymbolStyleLayer.textFont
totextFontNames
The developer would expect to set this property to an NSFont or UIFont, or perhaps an NSFontDescriptor or UIFontDescriptor, but definitely not an array of strings.
textFontNames
is also consistent withNSFont.fontName
,UIFont.fontName
, andMGLSymbolStyleLayer.iconImageName
.MGLSymbolStyleLayer.textSize
totextFontSize
in anticipation of Rename text-size to text-font-size mapbox-gl-style-spec#650Contrary to the property’s current name, it does not specify the dimensions of the overall label the way
text-max-width
does. Instead, it specifies the font size in pixels. This inconsistency means a developer would be tempted to set the property to aCGSize
orNSSize
value instead of anNSNumber
.MGLCircleStyleLayer.circlePitchScale
tocircleScaleAlignment
, in anticipation of Rename circle-pitch-scale to circle-scale-alignment mapbox-gl-style-spec#645The developer would expect to set this property to a
CGFloat
, especially considering that we renamedMGLSymbolStyleLayer.iconScale
(renamed fromiconSize
) is numeric.MGLCircleStyleLayer.circleTranslate
tocircleTranslation
,circleTranslateAnchor
tocircleTranslationAnchor
,MGLCircleTranslateAnchor
toMGLCircleTranslationAnchor
, and the corresponding fill, line, and symbol properties and enumerations, to complete the work done in Use appropriate part of speech for properties #7457Of these changes,
MGLSymbolStyleLayer.text
is perhaps the most disruptive, but I also see the upstream property inevitably getting renamed to something or other.Fixes #6098 once and for all. Past this point, if we ever rename any properties unilaterally in the iOS and macOS SDKs, we should not only declare the old name but make it available (albeit deprecated), to minimize disruption. But I’m pretty confident that this will be the last batch of renames for existing properties.
/cc @boundsj @friedbunny @ericrwolfe