mapbox / mapbox/mapbox-gl-js

Inconsistent formatting for event type properties

Open
#10,865 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

api :memo: docs :scroll:
Dominant language
TypeScript
Stars
12.4k
Forks
2.4k
PR merge metrics
No merged PRs in 30d

Description

In the [event types API reference page](https://docs.mapbox.com/mapbox-gl-js/api/events/), 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](https://github.com/mapbox/mapbox-gl-js/blob/main/src/ui/events.js). 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](https://user-images.githubusercontent.com/14878684/125844532-75fd5f66-f665-4ce6-b7bf-f99ec34d4db9.png)

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:

- List all of the properties with `@property`
- I think this syntax is cleanest in the source code.
- This would create UI issues with larger properties. Could we configure the doc to view properties in dropdowns?
- List all of the properties in individual comments
- Dropdowns for all properties might be an improvement.
- It doesn't seem technically correct to refer to TS type classes as "Instance Members."
- Keep the source as is but configure JSDoc to make the styling more consistent.
- E.g. both properties and instance members after the example and in dropdowns.
- I'm not sure what's possible here.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the event definitions in src/ui/events.js and compare the generated entries on the event types API reference page. Review how @property annotations and commented class properties become “properties” and “Instance Members,” then choose a consistent presentation that handles extensively documented properties. Done means the affected event types use the same understandable UI in the generated reference.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
documentation
Issue type
Documentation
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.