ASCOMInitiative / ASCOMRemote

The ASCOM REST based Remote Driver Server and Remote Access Clients
GNU General Public License v3.0
57 stars 15 forks source link

Various OpenAPI improvements #58

Closed RReverser closed 2 weeks ago

RReverser commented 3 weeks ago

Something I wanted to do during my last streak of Alpaca's OpenAPI PRs is use named enums instead of having all those 0 = variant1, 1 = variant2, ... manual descriptions in plain text.

Unfortunately, before OpenAPI 3.1 this was only possible to do via 3rd-party plugins - which I didn't want to tie Alpaca's OpenAPI definition to. Then it became possible to do via const + oneOf as described here, but Swagger UI didn't support those definitions yet.

Now that Swagger UI has implemented missing fetaures, everything has finally snapped in place.

This PR:

Some motivational screenshots before/after:


Before: image

After: image


Before: image

After: image


Before: image

After: image

RReverser commented 2 weeks ago

The ErrorNumber display is complex: "Integer (Integer | Number | Integer)" and I am concerned that it will confuse less experienced users. Is it possible to simplify it?

Fixed with a simple work around, so now it prints it as integer | (integer | integer).

RReverser commented 2 weeks ago

Ideally, I wish it also would deduplicate same types in that list instead of listing them over and over, but, sadly this is something that would have to be fixed in Swagger itself.

That said, it should be fairly easy to completely hide this field altogether with CSS. I figure the format (int32 / uint32) should be self-describing enough, and less confusing than showing both integer | (integer | integer) and int32?

RReverser commented 2 weeks ago

Just added said CSS. I think this looks much cleaner:

image

RReverser commented 2 weeks ago

One more small tweak - made constants have the same style as range constraints so that they're easier to spot.

image

Peter-Simpson commented 2 weeks ago

Thanks Ingvar, that looks so much better and cleaner, I'll merge the branch and add the Platform 7 changes.

RReverser commented 2 weeks ago

Just added said CSS. I think this looks much cleaner:

Only just noticed that this change also hid types like string as they don't have a fallback custom format to show. That's not ideal, I'm going to fix it in a follow-up PR in a second.