From 820f0e275ee5df3e7a2d4a36e11b8a68c946ed6b Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Thu, 26 Mar 2020 21:03:26 -0700 Subject: [PATCH] api docs: Redesign visuals for documenting arguments. The previous system for documenting arguments was very ugly if any of the examples or descriptions were wrong. After thinking about this for a while, I concluded the core problem was that a table was the wrong design element to use for API parameters, and we'd be much better off with individual card-type widgets instead. This rewrites the API arguments documentation implementation to use a basic sort of card-like system with some basic styling; I think the result is a lot more readable, and it's a lot more clear how we would add additional OpenAPI details (like parameter types) to the documentation. --- static/styles/portico/markdown.scss | 10 ++++ static/styles/portico/portico.scss | 27 +++++++--- .../bugdown/api_arguments_table_generator.py | 53 +++++++------------ zerver/openapi/zulip.yaml | 47 +++++----------- 4 files changed, 61 insertions(+), 76 deletions(-) diff --git a/static/styles/portico/markdown.scss b/static/styles/portico/markdown.scss index e1527c04e4..f234df3e0d 100644 --- a/static/styles/portico/markdown.scss +++ b/static/styles/portico/markdown.scss @@ -291,6 +291,16 @@ } } + code { + /* Copied from rendered_markdown.scss */ + font-size: 0.857em; + unicode-bidi: embed; + direction: ltr; + white-space: pre-wrap; + padding: 0px 4px; + background-color: hsl(0, 0%, 100%); + } + .code-section { ol { margin-left: 15px; diff --git a/static/styles/portico/portico.scss b/static/styles/portico/portico.scss index 94d8bff49b..82a7a39f7b 100644 --- a/static/styles/portico/portico.scss +++ b/static/styles/portico/portico.scss @@ -1717,14 +1717,27 @@ input.new-organization-button { padding-right: 10px; } -.table .json-api-example { - width: fit-content; - max-width: 300px; - word-wrap: break-word; -} +.api-argument { + .api-argument-name { + font-family: monospace; + } -.table .json-api-example code { - white-space: pre-wrap; + .api-argument-example-label { + font-style: italic; + } + + .api-argument-required { + text-transform: uppercase; + font-weight: 600; + font-size: 12px; + color: hsl(14, 75%, 60%); + } + .api-argument-optional { + text-transform: uppercase; + font-weight: 400; + font-size: 12px; + color: hsl(0, 0%, 47%); + } } .desktop-redirect-box { diff --git a/zerver/lib/bugdown/api_arguments_table_generator.py b/zerver/lib/bugdown/api_arguments_table_generator.py index f8c6b20d37..394df36c6e 100644 --- a/zerver/lib/bugdown/api_arguments_table_generator.py +++ b/zerver/lib/bugdown/api_arguments_table_generator.py @@ -1,6 +1,6 @@ import re import os -import ujson +import json from django.utils.html import escape as escape_html from markdown.extensions import Extension @@ -67,7 +67,7 @@ class APIArgumentsTablePreprocessor(Preprocessor): raise e else: with open(filename, 'r') as fp: - json_obj = ujson.load(fp) + json_obj = json.load(fp) arguments = json_obj[doc_name] if arguments: @@ -89,29 +89,17 @@ class APIArgumentsTablePreprocessor(Preprocessor): return lines def render_table(self, arguments: List[Dict[str, Any]]) -> List[str]: + # TODO: Fix naming now that this no longer renders a table. table = [] - beginning = """ - - - - - - - - - - -""" - tr = """ - - - - - - -""" - - table.append(beginning) + argument_template = """ +
+

{argument} {required}

+
+ Example: {example} +
+
{description}
+
+
""" md_engine = markdown.Markdown(extensions=[]) @@ -125,23 +113,20 @@ class APIArgumentsTablePreprocessor(Preprocessor): default = argument.get('schema', {}).get('default') if default is not None: - description += '\nDefaults to `{}`.'.format(ujson.dumps(default)) + description += '\nDefaults to `{}`.'.format(json.dumps(default)) - # TODO: Swagger allows indicating where the argument goes - # (path, querystring, form data...). A column in the table should - # be added for this. - table.append(tr.format( + # TODO: OpenAPI allows indicating where the argument goes + # (path, querystring, form data...). We should document this detail. + table.append(argument_template.format( argument=argument.get('argument') or argument.get('name'), # Show this as JSON to avoid changing the quoting style, which # may cause problems with JSON encoding. - example=escape_html(ujson.dumps(argument['example'])), - required='Yes' if argument.get('required') else 'No', + example=escape_html(json.dumps(argument['example'])), + required='required' if argument.get('required') + else 'optional', description=md_engine.convert(description), )) - table.append("") - table.append("
ArgumentExampleRequiredDescription
{argument}{example}{required}{description}
") - return table def makeExtension(*args: Any, **kwargs: str) -> MarkdownArgumentsTableGenerator: diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index e5474ac72e..b07c17bf12 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -1328,39 +1328,13 @@ paths: example: [ { - "id":1, - "value":"short text data" + "id": 4, + "value": "vim" }, { - "id":2, - "value":"long text data" + "id": 5, + "value": "1909-04-05" }, - { - "id":3, - "value":"short text data" - }, - { - "id":4, - "value":"vim" - }, - { - "id":5, - "value":"1909-3-5" - }, - { - "id":6, - "value":"https:\/\/zulipchat.com" - }, - { - "id":7, - "value":[ - 12 - ] - }, - { - "id":8, - "value":"cordelia" - } ] required: false @@ -2419,8 +2393,9 @@ paths: example: ['message'] - name: narrow in: query - description: A JSON-encoded array of length 2 indicating the narrow - for which you'd like to receive events for. For instance, to + description: | + A JSON-encoded array of length 2 indicating the narrow + for which you'd like to receive events. For instance, to receive events for the stream `Denmark`, you would specify `narrow=['stream', 'Denmark']`. Another example is `narrow=['is', 'private']` for private messages. @@ -2951,7 +2926,7 @@ paths: description: | Whether the stream is limited to announcements. - **Changes**: Deprecated in Zulip 2.2, use stream_post_policy instead. + **Changes**: Deprecated in Zulip 2.2, use `stream_post_policy` instead. schema: type: boolean example: true @@ -2965,7 +2940,7 @@ paths: * 2 => Only administrators can post. * 3 => Only new members can post. - **Changes**: New in Zulip 2.2, replacing the previous is_announcement_only + **Changes**: New in Zulip 2.2, replacing the previous `is_announcement_only` boolean. schema: type: integer @@ -2973,7 +2948,9 @@ paths: required: false - name: history_public_to_subscribers in: query - description: The new state for the history_public_to_subscribers. + description: | + Whether subscribers have access to full stream history, even before they joined + the stream. schema: type: boolean example: true