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

Override style specification documentation in runtime styling API#6107

Merged
frederoni merged 1 commit into
masterfrom
5954-documentation-overrides
Nov 8, 2016
Merged

Override style specification documentation in runtime styling API#6107
frederoni merged 1 commit into
masterfrom
5954-documentation-overrides

Conversation

@frederoni
Copy link
Copy Markdown
Contributor

@frederoni frederoni commented Aug 22, 2016 •
edited
Loading

Fixes #5954 - Override some gl-style-spec documentation to make it more idiomatic.

👀 @1ec5 @friedbunny

@frederoni frederoni added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS documentation runtime styling labels Aug 22, 2016
@frederoni frederoni added this to the ios-v3.4.0 milestone Aug 22, 2016
@frederoni frederoni added the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Aug 22, 2016
@frederoni frederoni force-pushed the 5954-documentation-overrides branch 7 times, most recently from 719c227 to d6ac418 Compare August 22, 2016 14:59
1ec5
1ec5 reviewed Aug 22, 2016
View reviewed changes
Comment thread platform/darwin/scripts/generate-style-code.js Outdated
${property.docs[property.values.indexOf(value)]}
*/`;
} else {
return "#warning documentation missing.";
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 Aug 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These warnings might be easier to track if they included the names of the enum values that are missing documentation.

@1ec5 1ec5 changed the title [ios, macos] outlined style spec documentation overrides Override style specification documentation in runtime styling API Sep 14, 2016
@boundsj boundsj mentioned this pull request Sep 26, 2016
Closed
@incanus incanus mentioned this pull request Sep 27, 2016
Closed
@1ec5
Copy link
Copy Markdown
Contributor

1ec5 commented Sep 30, 2016

All the #warnings for enum values will conflict (but can go away) once #6508 lands. 🎉

/cc @incanus

@1ec5 1ec5 mentioned this pull request Sep 30, 2016
Closed
@boundsj boundsj added the release blocker Blocks the next final release label Oct 28, 2016
@frederoni frederoni force-pushed the 5954-documentation-overrides branch 2 times, most recently from 2b690a5 to 32c93dd Compare November 2, 2016 14:21
@frederoni
Copy link
Copy Markdown
Contributor Author

frederoni commented Nov 2, 2016 •
edited
Loading

Rebased and cleaned. icon-text-fit-padding was the only style spec documentation I could find that contradicts itself. @1ec5 👀

1ec5
1ec5 suggested changes Nov 2, 2016
View reviewed changes
Comment thread platform/darwin/scripts/generate-style-code.js Outdated
}

global.override = function(base, overrides) {
Object.keys(overrides).forEach(function(key) {
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 Nov 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be expressed as a for-of loop:

for (var key of overrides) { … }

Comment thread platform/darwin/scripts/generate-style-code.js Outdated
return (typeof object === 'object' && !Array.isArray(object));
}

global.override = function(base, overrides) {
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 Nov 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of rolling our own function, import lodash and use _.assign() or _.assignIn().

Comment thread platform/darwin/scripts/style-spec-overrides-v8.json Outdated
{
"layout_symbol": {
"icon-text-fit-padding": {
"doc": "Size of the additional area added to dimensions determined by `icon-text-fit`, in counterclockwise order: top, left, bottom, right."
Copy link
Copy Markdown
Contributor

@1ec5 1ec5 Nov 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

Padding added to the dimensions determined by the iconTextFit property.

We already mention in the generated documentation that the NSValue contains an NSEdgeInsets or UIEdgeInsets. The developer can look up what the expected order is for those types.

@1ec5
Copy link
Copy Markdown
Contributor

1ec5 commented Nov 2, 2016 •
edited by frederoni
Loading

Some additional properties that we should fix:

  • Instead of “sprite”, which is a term we’re trying to phase out, refer to “style images” and -[MGLStyle setImage:forName:].
    • MGLBackgroundStyleLayer.backgroundPattern
    • MGLFillStyleLayer.fillPattern
    • MGLLineStyleLayer.linePattern
  • Remove the “Values are…” sentence, which talks about an array; moreover, the negative Y part will be incorrect for macOS (Offsets should be flipped vertically on macOS #6066). The developer can always refer to CGVector documentation.
    • MGLCircleStyleLayer.circleTranslate
    • MGLFillStyleLayer.fillTranslate
    • MGLLineStyleLayer.lineTranslate
  • Remove the “Positive values…” sentence, which will be incorrect on macOS.
    • MGLSymbolStyleLayer.iconOffset
    • MGLSymbolStyleLayer.textOffset
    • MGLSymbolStyleLayer.iconTranslate
    • MGLSymbolStyleLayer.textTranslate
  • Remove “as rgba”, which is CSS syntax. It’s perfectly acceptable to use UIColor(hue:saturation:brightness:alpha:) or UIColor(white:alpha:), after all.
    • MGLFillStyleLayer.fillColor
  • Remove “similar to the CSS text-transform property”.
    • MGLTextTransform
    • MGLSymbolStyleLayer.textTransform

It may be possible to fix these properties without an override, similar to how we replace “pixels” with “points”.

@frederoni frederoni force-pushed the 5954-documentation-overrides branch 2 times, most recently from cfb587e to 7c768b6 Compare November 7, 2016 12:21
@frederoni frederoni force-pushed the 5954-documentation-overrides branch from 7c768b6 to b70babe Compare November 7, 2016 12:23
@frederoni frederoni removed the ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold label Nov 7, 2016
@frederoni frederoni force-pushed the 5954-documentation-overrides branch from b70babe to 792d75b Compare November 8, 2016 12:23
@frederoni frederoni merged commit 5a828cb into master Nov 8, 2016
@1ec5 1ec5 mentioned this pull request Dec 9, 2016
Merged
@jfirebaugh jfirebaugh deleted the 5954-documentation-overrides branch December 12, 2016 18:48
@1ec5 1ec5 mentioned this pull request Mar 16, 2017
Merged
12 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@1ec5 1ec5 1ec5 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@frederoni frederoni

Labels

documentation iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS release blocker Blocks the next final release runtime styling

Projects

None yet

Milestone

ios-v3.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@frederoni @1ec5 @boundsj