mapbox / mapbox-gl-js

Interactive, thoroughly customizable maps in the browser, powered by vector tiles and WebGL
https://docs.mapbox.com/mapbox-gl-js/
Other
11.11k stars 2.21k forks source link

Inconsistent formatting for event type properties #10865

Open SnailBones opened 3 years ago

SnailBones commented 3 years ago

In the event types API reference page, some event types list their properties as "properties" (MapBoxZoomEvent, MapDataEvent) and others as "Instance Members" (MapTouchEvent, MapMouseEvent, MapWheelEvent). They have considerably different UI, with the former appearing as text before the example and the latter as a list of dropdown boxes below it.

This is because they have inconsistent JSdoc labels in the source. MapBoxZoomEvent and MapDataEvent (both custom TS types) are annotated with the @property JSdoc label. Example:

 * @typedef {object} MapBoxZoomEvent
 * @property {MouseEvent} originalEvent The DOM event that triggered the boxzoom event. Can be a `MouseEvent` or `KeyboardEvent`.

This appears like:

image

The other three events are classes, and are annotated with a comment over each property:

    /**
     * The DOM event which caused the map event.
     */
    originalEvent: TouchEvent;

This appears as the following:

Screen Shot 2021-07-14 at 12 52 23 PM

As a further complication, some properties have extensive documentation including their own examples.

image

Most users presumably don't care if they're working with TS types or classes and would benefit from consistency in the documentation.

Three ideas about how to fix this:

danswick commented 3 years ago

Thanks for mapping this out, @SnailBones. I agree that your first option is probably the cleanest. I have always found the "instance members" language to be a bit of a case of inside baseball since, as I understand it, the term has more to do with the class structure of the source code than the way users will actually interact with the API in the real world.

Would we still need to keep the instance member code in tact for Flow? I don't totally understand how that works or whether we can get JSDoc / documentation.js to selectively ignore parts of it.

SnailBones commented 3 years ago

Thanks @danswick!

Would we still need to keep the instance member code in tact for Flow?

I don't think the comments have anything to do with Flow. They may provide help for developers using certain IDE extensions, but at least with my setup the @property syntax is better for that. So I'm in agreement that the first option would be preferable!

What are your thoughts about dropdowns vs lists of properties?

danswick commented 3 years ago

@SnailBones as @colleenmcginnis would say, I don't have a strong preference either way as long as we're consistent! IMO, the biggest positive of dropdowns is keeping the page structure more obvious, while lists are easier to control+F. In the past we've opted for page usability / navigation over control+F'ability, so I guess dropdowns would be more consistent with past decisions.

SnailBones commented 3 years ago

Can we get JSDoc to make dropdowns for @property and get the best of both worlds?