swagger-api / swagger-ui

Swagger UI is a collection of HTML, JavaScript, and CSS assets that dynamically generate beautiful documentation from a Swagger-compliant API.
https://swagger.io
Apache License 2.0
26.67k stars 8.97k forks source link

swagger-ui does not render all Parameter-fields #808

Closed ikitommi closed 9 years ago

ikitommi commented 9 years ago

enum seems to get nice docs, but at least pattern doesn't resolve in the swagger-ui. Would be nice to know which ones are supported or get support for them all.

and thanks for the great job with swagger-ui!

https://github.com/swagger-api/swagger-spec/blob/master/versions/2.0.md#parameter-object-

bshamblen commented 9 years ago

If nobody's currently working on this I'd be happy to add these. There's also a bug with how arrays are displayed, which I've already fixed but haven't submitted a pull request for.

webron commented 9 years ago

I think it's important to discuss first how it would be viewed in the UI. This has potential to overload it. There are two separate issues in this one - display and validation/enforcement.

The validation is probably clear enough to validate. The display, not so much. Do you have a suggestion on how to do it? @fehguy may have something in mind already.

bshamblen commented 9 years ago

I agree that adding all of these in the Model definition could cause it to become rather loud. One option would be to add a small icon to the end of the property if there are validation values and display the list in a popup on mouseover (similar to the oauth scope icon). It would prevent UI clutter, but still allow users to see the values.

webron commented 9 years ago

I was thinking of something similar. Possibly with on-hover tooltip?

By the way, for the arrays display issue, it'll probably be better to create a separate PR. And thank you for the contributions.

webron commented 9 years ago

By the way, this would work well for primitives (or single line entries such as path, query, header, form parameters), but may not work so well for complex models descriptions.

bshamblen commented 9 years ago

@webron I submitted a separate PR to address the way array types are displayed in the model/schema sections (as mentioned above). I'll wait until @fehguy has a chance to weigh in with his UI ideas to display the validation options before doing anything else. Thanks!

bshamblen commented 9 years ago

I added a hover over to each property in the Model section. It displays a pop up with the name of the property and any of the options listed in this issue. I currently only have it working to for a few of the options, just as a proof of concept. Here's a screen capture of the hover over in action.

https://www.flickr.com/x/t/0095009/gp/23972840@N04/GTKy17

If you're ok with how this looks I'll finish the remaining options.

I'm not sure if adding validation in the UI makes sense in this case. I would leave it up to the API server to handle the validation, since it has to be done anyway. Adding the validation here would only prevent someone from testing an invalid value against the API, in case they want to see an actual error response.

webron commented 9 years ago

@bshamblen - thank you for all the work you put into it. I think it looks good. I'm not UX expert and in time we may want to implement it differently, but that shows the information there and it seems to cover it.

As for the validation, you raise an interesting point. Right now, required fields are required by the ui (and that's actually part of the validation). I wonder if we should allow turning it on and off with a parameter. Developers and QA could definitely benefit from it being off, but when it's published documentation, you may want to keep it on.

ikitommi commented 9 years ago

+1 on making things parametrized, type-based render/validation fns for example. Would lovr too se a decent datepicker for dates. See #804. The extra info looks great!

fehguy commented 9 years ago

I agree. I would love to get this:

https://github.com/swagger-api/swagger-ui/issues/833

Handled then we can get stuff like this merged quickly.

bshamblen commented 9 years ago

I have the UI done for displaying all of these potential values and I could submit a PR now. Unfortunately I won't have time for a few weeks to extend the functionality to include validation. Please let me know if you'd like me to wait or submit now and do the validation as a separate PR.

webron commented 9 years ago

There's no reason this can't be two separate PRs, one for UI and one for functionality. I believe the community would appreciate any progress done here, so it would be great.

fehguy commented 9 years ago

moving this to m2

nickretallack commented 9 years ago

I put a lot of useful stuff in format. It would be nice if it showed up in the UI.

webron commented 9 years ago

Since the hover info was merged, what's currently missing here?

ikitommi commented 9 years ago

haven't tested all of those, but guess you have. From my part, this can be closed. thanks!

fehguy commented 9 years ago

We just added password. We're going to be waiting on https://github.com/swagger-api/swagger-js/issues/219

webron commented 9 years ago

Since we've made progress, closing this one for now. If needed, we can open a new issue to cover additional aspects.