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

[Issue 1258] Add local libs and fix a11y issues #1346

Merged
merged 3 commits into from
Mar 5, 2024

Conversation

acouch
Copy link
Collaborator

@acouch acouch commented Feb 27, 2024

Summary

Fixes #1258

Time to review: 10 mins

Changes proposed

  1. Pull the CSS and JS files from a CDN to the API container
  2. Fix two a11y issues:

Note about files:

Three files were moved to the API from the following sources:

The first two are unchanged. The css file had the following added in order to fix the contrast ratio:

.swagger-ui .info .title small {
    background-color: black;
}
.swagger-ui .opblock.opblock-post .opblock-summary-method,
.swagger-ui .info .title small.version-stamp {
    background-color: #48600d;
}
.swagger-ui .opblock.opblock-get .opblock-summary-method {
    background-color: #001b44;
}
.swagger-ui .json-schema-2020-12-expand-deep-button,
.swagger-ui .info a {
    color: darkblue;
    text-decoration: underline;
}
.swagger-ui .btn.authorize {
    color: #48600d;
    border-color: #48600d;
}
.swagger-ui .btn.authorize svg {
    fill: #48600d;
}

This is a temporary solution as we will move the API docs before the Search UI is public.

Context for reviewers

None of the OpenAPI doc generators seem to be 508 compliant out of the box 😢

Additional information

Before:

lighthouse screenshot showing multiple flags

After:

lighthouse showing 100 score

@acouch acouch force-pushed the acouch/1258-local-static-assets-accessibility branch from aaad91f to e7d383e Compare February 27, 2024 14:41
app.config["SWAGGER_UI_BUNDLE_JS"] = "/static/swagger-ui-bundle.js"
app.config["SWAGGER_UI_STANDALONE_PRESET_JS"] = "/static/swagger-ui-standalone-preset.js"
# Removing because the server dropdown has accessibility issues.
app.config["SERVERS"] = "."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this break the ability to use OpenAPI? The server info is currently automatically set for us, but if we manually set it, that might break querying entirely through the UI.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tested that it doesn't:

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to review the API Flask code to make sure there aren't side effects for this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Its not going to be in APIFlask, probably something in apispec. We probably can't tell if this will be an issue until it is in dev as the server logic is going to be a lot simpler locally

@chouinar
Copy link
Collaborator

From the summary This is a temporary solution as we will move the API docs before the Search UI is public. - What do you mean by this?

@acouch
Copy link
Collaborator Author

acouch commented Feb 27, 2024

@chouinar I updated the description to link to #1336

@acouch acouch merged commit 7c064b1 into main Mar 5, 2024
@acouch acouch deleted the acouch/1258-local-static-assets-accessibility branch March 5, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Audit and Fix Accessibility Issues with API Docs
3 participants