fixes #497: document enum values #510

incanus · 2016-09-28T19:01:44Z

Affects a bunch of downstream stuff which is being documented in #497 proper.

Also removes color operations in 44da3a8 per chat with @samanpwbb since these were cut back in #308.

jfirebaugh · 2016-09-28T19:12:30Z

reference/v8.json

-      ],
+      "values": {
+        "map": {
+            "doc": "When `symbol-placement` is set to `point`, produces icons whose x-axes are aligned east-west."


What about symbol-placement = line?

Whoops, de06217.

jfirebaugh · 2016-09-28T19:13:40Z

reference/v8.json

-      ],
+      "values": {
+        "map": {
+            "doc": "When `symbol-placement` is set to `point`, produces glyphs whose x-axes are aligned east-west."


jfirebaugh · 2016-09-28T19:16:36Z

reference/v8.json

-    ],
+    "values": {
+      "==": {
+          "doc": "The equal-to operator."


Let's pull in the descriptions from https://www.mapbox.com/mapbox-gl-style-spec/#types-filter for these.

Do you mean literally, as in set inclusion: feature[key] ∈ {v0, ..., vn}?

How about db47134?

Another option is to use English, for example:

A feature is included when the specified property is one of the specified values.

I think the problem with just saying “equal-to operator” is that it isn’t clear that the left-hand side must be the property name and the right-hand side must be a property value. Even saying “left” and “right” could be confusing, though, given that the JSON representation is ["==", "key", "value"] rather than ["key", "==", "value"].

I think this is pretty clear in the context of GL styling, or if not, is very easily fixed (flip things, using the format key == value).

I disagree. Equality is the most straightforward of the operators, but for example describing has as “the existence operator” doesn’t elucidate it for anyone who’s new to filters. How about something like this:

The inequality operator: <var>feature[key]</var> ≠ <var>value</var>.

Yes, that's what I meant by "pull in the descriptions from https://www.mapbox.com/mapbox-gl-style-spec/#types-filter". I think it would also be good to set up the key and value variables as the existing documentation does:

Trying this and testing downstream implications in 5848d76.

Downstream isn't affected, since we handle filter operators manually in the SDKs. How do you feel about this @1ec5 @jfirebaugh?

1ec5 · 2016-09-28T19:20:27Z

reference/v8.json

+          "line": {
+              "doc": "The label is placed along the line of the geometry."
+          }
+      },
      "default": "point",
      "doc": "Label placement relative to its geometry. `line` can only be used on LineStrings and Polygons.",


Let’s move the second sentence to the line documentation.

1ec5 · 2016-09-28T19:21:43Z

reference/v8.json

+            "doc": "The text is aligned to the plane of the viewport."
+        },
+        "auto": {
+            "doc": "Set to the same value as `text-rotation-alignment`."


Automatically matches the value of text-rotation-alignment.

1ec5 · 2016-09-28T19:23:20Z

reference/v8.json

+            "doc": "The bottom of the text is placed closest to the anchor."
+        },
+        "top-left": {
+            "doc": "The top left side of the text is placed closest to the anchor."


s/top left side/top-left corner/

1ec5 · 2016-09-28T19:25:12Z

reference/v8.json

+            "doc": "The text is not altered."
+        },
+        "uppercase": {
+            "doc": "The text is uppercased."


Forces all letters to be displayed in uppercase.

This way there’s no ambiguity about capitalize-style behavior.

1ec5 · 2016-09-28T19:25:44Z

reference/v8.json

      "default": "visible",
-      "doc": "The display of this layer. `none` hides this layer.",
+      "doc": "The display of this layer.",


Whether this layer is displayed.

1ec5 · 2016-09-28T19:30:29Z

reference/v8.json

-      "doc": "Control whether the translation is relative to the map (north) or viewport (screen)",
+      "values": {
+        "map": {
+            "doc": "The fill is translated relative to the map (north)."


What does “north” mean here? Does it mean that a higher y value moves the fill’s origin to the north?

1ec5 · 2016-09-28T19:32:00Z

reference/v8.json

+            "doc": "The fill is translated relative to the map (north)."
+        },
+        "viewport": {
+            "doc": "The fill is translated relative to the viewport (screen)."


What does “screen” mean here? Does it mean that a higher y value moves the fill’s origin up? Or down? (I suppose specifying one or the other would be problematic for platform-specific documentation such as mapbox/mapbox-gl-native#6066, but that isn’t the style specification’s problem.)

samanpwbb · 2016-09-28T20:29:46Z

In order to shorten the descriptions, can we cut the "when set to {val}" part of all the descriptions? I don't think it's necessary.

jfirebaugh · 2016-09-28T20:34:16Z

They're all supposed to read "When symbol-placement is set to...", referring to the behavior of other properties in the presence of symbol-placement. There's a couple of places where "symbol-placement is" is omitted where that's unclear that we should fix, but we can't remove that text entirely.

jfirebaugh · 2016-09-28T20:39:21Z

reference/v8.json

-      ],
+      "values": {
+        "none": {
+            "doc": "No scaling is performed."


"The icon is displayed at its intrinsic aspect ratio." (icon-size can still scale it.)

jfirebaugh · 2016-09-28T20:40:17Z

reference/v8.json

+        "none": {
+            "doc": "No scaling is performed."
+        },
+        "both": {


Make this the last property, so they are documented in a natural order.

jfirebaugh · 2016-09-28T20:40:42Z

reference/v8.json

+            "doc": "Scaling is done in both x- and y-dimensions."
+        },
+        "width": {
+            "doc": "Scaling is done in the x-dimension to fit the text's dimensions."


"The icon is scaled in the x-dimension to fit the width of the text."

samanpwbb · 2016-09-28T20:34:42Z

reference/v8.json

-      ],
+      "values": {
+        "map": {
+            "doc": "When `symbol-placement` is set to `point`, produces glyphs whose x-axes are aligned east-west. When set to `line`, produces glyphs whose x-axes are aligned with the line."


produces glyphs whose x-axes are aligned east-west

"produces glyphs who x-axes" is unnecessarily techincal. This could benefit from plainer language.

Point: Aligns text east-west.
Line: Places text along corresponding line geometry.

Point: Aligns text east-west.

👍

Line: Places text along corresponding line geometry.

There's an important nuance that this doesn't convey; it's the difference between:

╱ ↗ ╱ ↗ ╱

and

╱ → ╱ → ╱

incanus · 2016-09-28T22:55:08Z

There's a couple of places where "symbol-placement is" is omitted where that's unclear that we should fix

ca7b446

incanus · 2016-09-28T23:03:16Z

~~Ready for re-review @1ec5 @jfirebaugh @samanpwbb.~~

Oops, still need to hit doc generator etc.

incanus · 2016-09-28T23:12:21Z

Doc generator grabbed in 674a1e1.

Do we want to update the page layout now after this? Now we have:

However, we don't yet capitalize on our new enum documentation. Should we in this context?

jfirebaugh · 2016-09-28T23:16:52Z

Yes, let's stick them in a <dl>.

incanus · 2016-09-29T01:31:41Z

Ok, ready for review again. Downstream work is happening in mapbox/mapbox-gl-native#6508 and is looking good as well.

jfirebaugh · 2016-09-29T18:41:59Z

incanus · 2016-09-29T22:47:40Z

@1ec5 Mind re-reviewing and unblocking this?

1ec5 · 2016-09-29T23:47:20Z

reference/v8.json

-    ],
+    "values": {
+      "==": {
+          "doc": "The equality operator."


Can we be any more specific? The equality operator ==, as most programmers know it, allows the left and right sides to be reversed. That isn't the case with any of these operators.

#510 (comment)

Documentation for enum values landed in mapbox/mapbox-gl-style-spec#510. This updates Android, iOS, and macOS documentation code gen scripts to capitalize on them.

incanus added 11 commits September 28, 2016 11:46

data sources

f39eba9

geometry types

99ef21b

layer display

996b8a4

line cap & join

d97d180

placement

029b14a

alignment, translation, scaling, anchor, justification

04c8fb7

text transformation

b6d542d

filter operators & geometries

0cecf5d

function interpolation

0471e09

remove dead color operations

44da3a8

minor formatting tweaks

d6c5510

incanus added the documentation label Sep 28, 2016

incanus self-assigned this Sep 28, 2016

incanus added the in progress label Sep 28, 2016

jfirebaugh suggested changes Sep 28, 2016

View reviewed changes

fix missing line description

de06217

1ec5 suggested changes Sep 28, 2016

View reviewed changes

align filter operator descriptions with online docs

db47134

jfirebaugh reviewed Sep 28, 2016

View reviewed changes

samanpwbb reviewed Sep 28, 2016

View reviewed changes

incanus added 6 commits September 28, 2016 14:55

validate both scalar & object enum values

5422987

fix indentation and implicit parentheses

cbca231

split up sections, add comments, add parentheses

b6d9220

update validation for enum value docs

4e0932a

update enum validation for objects

10aab62

refresh minified v8

cecad91

incanus added 2 commits September 28, 2016 15:52

better contextual placement

742bc7a

clarify relationship

ca7b446

incanus added 5 commits September 28, 2016 15:55

more accurate

6b79dae

rearrange to natural order

3477118

scaling clarifications

6b8c8e2

plainer text

ab29a11

bump minified v8

0795459

update docs enum generator

674a1e1

incanus added 2 commits September 28, 2016 16:46

tweaks to online docs for enum values

77826c0

change to literal values

cc63f58

incanus mentioned this pull request Sep 29, 2016

update style generation code for new style spec enum values docs mapbox/mapbox-gl-native#6508

Merged

3 tasks

jfirebaugh approved these changes Sep 29, 2016

View reviewed changes

fix typo

95d98f6

1ec5 suggested changes Sep 29, 2016

View reviewed changes

incanus added 2 commits September 30, 2016 12:07

better filter operation docs

5848d76

Merge remote-tracking branch 'origin/mb-pages' into 497-enum-value-docs

0c01dde

1ec5 approved these changes Sep 30, 2016

View reviewed changes

incanus merged commit 7f62a4f into mb-pages Sep 30, 2016

incanus removed the in progress label Sep 30, 2016

incanus deleted the 497-enum-value-docs branch September 30, 2016 21:51

incanus mentioned this pull request Sep 30, 2016

Document enum values individually #497

Closed

davidtheclark mentioned this pull request Oct 4, 2016

Add fill-extrude-height, fill-extrude-base, and light root property #495

Merged

fixes #497: document enum values #510

fixes #497: document enum values #510

Conversation

incanus commented Sep 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ec5 Sep 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samanpwbb commented Sep 28, 2016

jfirebaugh commented Sep 28, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samanpwbb Sep 28, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

incanus commented Sep 28, 2016

incanus commented Sep 28, 2016 • edited Loading

incanus commented Sep 28, 2016

jfirebaugh commented Sep 28, 2016

incanus commented Sep 29, 2016

jfirebaugh commented Sep 29, 2016

incanus commented Sep 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

incanus commented Sep 28, 2016 •

edited

Loading

1ec5 Sep 28, 2016 •

edited

Loading

samanpwbb Sep 28, 2016 •

edited

Loading

incanus commented Sep 28, 2016 •

edited

Loading