Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small quality of life fixes #941

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sanscontext
Copy link

@sanscontext sanscontext commented Jul 12, 2023

This PR:

  • ✔️ Adds a link to the #auth anchor on the "no api key set" message, so that readers get a CTA how to fix it.
    After: (I don't have a non-edited version to screenshot for a Before)
    image

  • ✔️ Renders the bearerFormat string as the field hint text, for HTTP Bearer type auth, if one is set. (If not set, uses the original api-token.)
    Before:
    image
    After:
    image

  • ✔️ Similarly, renders the bearerFormat string as the noun in the instructional text, if set. (If not set, uses the original "Token String".)
    Before:
    image
    After:
    image

  • ⚠️ Attempts to check if there are >1 of a type of security scheme, and add the ${v.securitySchemeId} if there are, except I couldn't get this working or troubleshoot effectively with my rudimentary JS skills.
    Not working right now, but when it's done it would look something like this:
    Before: two different auth formats with the same name, so you have to use the descriptive text to tell them apart.
    image
    After: much easier to see that these are different
    image

Thanks all!

@AdrianMachado
Copy link
Contributor

Screenshots of before and after would be appreciate @sanscontext :)

@AdrianMachado
Copy link
Contributor

cc @mrin9

@sanscontext
Copy link
Author

Thanks @AdrianMachado , I added some before/after screenshots, trying to limit it to just what I've touched in this PR.

I was thinking about the bearerFormat part, and I'd actually want to see some examples of how it's used in the wild before assuming it'd be helpful here. The OAS isn't very prescriptive on how this should be used, and "Hint" isn't going to mean the same thing to everyone. Would this be something we could poll the Discord for, for examples?

@mrin9
Copy link
Collaborator

mrin9 commented Jul 17, 2023

✔️ Adds a link to the #auth anchor on the "no api key set" message, so that readers get a CTA how to fix it.
After: (I don't have a non-edited version to screenshot for a Before)

This may not be the case always. An user can always hide the auth section, in that case the link wont take it to anywhere

@@ -401,7 +401,7 @@ export default function securitySchemeTemplate() {
<div class="blue-text"> ${providedApiKeys.length} API key applied </div>
<div style="flex:1"></div>
<button class="m-btn thin-border" part="btn btn-outline" @click=${() => { onClearAllApiKeys.call(this); }}>CLEAR ALL API KEYS</button>`
: html`<div class="red-text">No API key applied</div>`
: html`<div class="red-text">No API key applied - <a href="#auth">Apply one here</a></div>`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#auth wont be available always.it depends on which section the user is showing and also special care must be taken for focused view

@sanscontext
Copy link
Author

Thanks all, I'm open to additions to this PR. As I mentioned to @AdrianMachado, I'm not a developer so my changes here are rudimentary but high-impact on our end. It'd be nice not to have to add them to every new version of Rapidoc I pull down.

@AdrianMachado
Copy link
Contributor

I can take a look later this week

@sanscontext
Copy link
Author

Hey folks, it's been six months. Should I just close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants