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

Fix Swagger UI colors for better Accessibility #3887

Merged
merged 4 commits into from Sep 8, 2021

Conversation

smridge
Copy link
Contributor

@smridge smridge commented Aug 7, 2021

This PR resolves Accessibility issues for form fields of the swagger-ui. Colors used from current theme and validated in contrast checker. I'm leaning towards calling this a bug as the input fields are nearly impossible to see.

Contrast Checker Before:

Contrast Checker After:

  • input fields and label fields pass
  • gray button border pass for user interface components. btn:not(.btn--plain) was overriding the original .swagger .btn, this returns border to the expected behavior.

Examples

Before After
before after
Before After
before-2 after-2
Before After
before-3 after-3

@reynolek
Copy link
Contributor

reynolek commented Sep 5, 2021

Hey @smridge -- Thanks for opening this PR! Out of curiosity did you go through any of the other themes for accessibility or was the focus primarily just on Core Default? Just want to make sure I test the changes properly.

@vercel vercel bot temporarily deployed to Preview September 5, 2021 03:39 Inactive
@reynolek
Copy link
Contributor

reynolek commented Sep 5, 2021

This does indeed do what you set out to do (for the 1 theme). However my gut tells me that we probably should look at extending the theming process to give us more granular control over swagger-ui via the theme (i.e. make it a first class object in the theme rather than raw css) -- I would be interested in the opinion of @develohpanda here.

I spent a little bit of time tonight looking at how the theme generation works and was having some problems, but think I am pretty close to making that work and being able to give guidance if thats the approach other maintainers feel we should take.

@develohpanda
Copy link
Contributor

develohpanda commented Sep 5, 2021

The before/after images look great for sure, thanks a tonne for doing this!

we probably should look at extending the theming process to give us more granular control over swagger-ui via the theme (i.e. make it a first class object in the theme rather than raw css)

My gut feeling says we should avoid adding a first-class object specifically for Swagger UI, but figure out a way to use the existing variables where possible (so I agree with parts of this, haha). I'd like to avoid a first-class object because we might paint ourselves into a corner by adding a proxy that we maintain between Swagger UI's CSS and plugin theme developers. If/when the 3rd party package (Swagger UI) change up their CSS, we'll first need to update our proxy and then have plugin developers update their plugins.

On the other hand, if we keep the raw css around, should we ever replace the Swagger UI preview with a different preview, none of the existing plugins that override Swagger UI styling will work out of the box. But I think that's okay because as it stands, plugins override swagger specific classes already and wouldn't expect other previews to work.


Here's what I think we should explore. We do connect the Swagger UI Preview into our existing theme variables using https://github.com/Kong/insomnia/blob/3b757251c9a09eeec9b392f30b55e2796afa0664/packages/insomnia-app/app/ui/css/components/spec-editor.less. This is by no means complete, but it would be nice to translate the CSS in this PR into this file, using existing variables and connecting them into Swagger UI, like the other existing examples.

This way, we improve how we connect existing theme variables with the Swagger UI, and it should fix contrast issues in all themes, while also not exposing a direct Swagger styling proxy to plugin developers. If this proves too tricky (I'm just ideating at this stage), we can explore other routes too!

How does that sound?

@reynolek
Copy link
Contributor

reynolek commented Sep 5, 2021

When I was playing with this idea, I was looking to update spec-editor.less -- The one thing I noticed across different specs is that because of the way the actual swagger CSS is, it is set to inherit the color, and any themes that have a closer-to-white color for --color-font (aka default for the foreground in the theme) exhibits the same issue as what this PR aims to improve.

My thought was to expose something like editorFont or something in the style section of the theme, and then update spec-editor.less to use that color.

That being said I couldnt get it all to wire up properly so I may have been doing something wrong.

@develohpanda
Copy link
Contributor

develohpanda commented Sep 6, 2021

@reynolek / @smridge, I just pushed 219f883 (#3887), which translates the raw css from the default theme into the spec-editor.less to work with any theme. As I compare this commit with the "after" screenshots in the description, they all seem to still match up. Let me know if this is not the case! It derives from the input styles used by form elements in the app.

The one thing I noticed across different specs is that because of the way the actual swagger CSS is, it is set to inherit the color, and any themes that have a closer-to-white color for --color-font (aka default for the foreground in the theme) exhibits the same issue as what this PR aims to improve.

@reynolek could you please share some examples where this is the case?

Some screenshots here:

image

image

image

I used the following spec to test

spec
openapi: 3.0.0
info:
  description: "This is a sample server Petstore server.  You can find out more about
    Swagger at [http://swagger.io](http://swagger.io) or on [irc.freenode.net,
    #swagger](http://swagger.io/irc/).  For this sample, you can use the api key
    `special-key` to test the authorization filters."
  version: 1.0.2
  title: Swagger Petstore
  termsOfService: http://swagger.io/terms/
  contact:
    email: apiteam@swagger.io
  license:
    name: Apache 2.0
    url: http://www.apache.org/licenses/LICENSE-2.0.html
tags:
  - name: pet
    description: Everything about your Pets
    externalDocs:
      description: Find out more
      url: http://swagger.io
  - name: store
    description: Access to Petstore orders
  - name: user
    description: Operations about user
    externalDocs:
      description: Find out more about our store
      url: http://swagger.io
paths:
  /pet:
    post:
      tags:
        - pet
      summary: Add a new pet to the store
      description: ""
      operationId: addPet
      requestBody:
        $ref: "#/components/requestBodies/Pet"
      responses:
        "405":
          description: Invalid input
    put:
      tags:
        - pet
      summary: Update an existing pet
      description: ""
      operationId: updatePet
      requestBody:
        $ref: "#/components/requestBodies/Pet"
      responses:
        "400":
          description: Invalid ID supplied
        "404":
          description: Pet not found
        "405":
          description: Validation exception
  /pet/findByStatus:
    get:
      tags:
        - pet
      summary: Finds Pets by status
      description: Multiple status values can be provided with comma separated strings
      operationId: findPetsByStatus
      parameters:
        - name: status
          in: query
          description: Status values that need to be considered for filter
          required: true
          explode: true
          schema:
            type: array
            items:
              type: string
              enum:
                - available
                - pending
                - sold
              default: available
      responses:
        "200":
          description: successful operation
          content:
            application/xml:
              schema:
                type: array
                items:
                  $ref: "#/components/schemas/Pet"
            application/json:
              schema:
                type: array
                items:
                  $ref: "#/components/schemas/Pet"
        "400":
          description: Invalid status value
  /pet/findByTags:
    get:
      tags:
        - pet
      summary: Finds Pets by tags
      description: Multiple tags can be provided with comma separated strings. Use tag1,
        tag2, tag3 for testing.
      operationId: findPetsByTags
      parameters:
        - name: tags
          in: query
          description: Tags to filter by
          required: true
          explode: true
          schema:
            type: array
            items:
              type: string
      responses:
        "200":
          description: successful operation
          content:
            application/xml:
              schema:
                type: array
                items:
                  $ref: "#/components/schemas/Pet"
            application/json:
              schema:
                type: array
                items:
                  $ref: "#/components/schemas/Pet"
        "400":
          description: Invalid tag value
      deprecated: true
  "/pet/{petId}":
    get:
      tags:
        - pet
      summary: Find pet by ID
      description: Returns a single pet
      operationId: getPetById
      parameters:
        - name: petId
          in: path
          description: ID of pet to return
          required: true
          schema:
            type: integer
            format: int64
      responses:
        "200":
          description: successful operation
          content:
            application/xml:
              schema:
                $ref: "#/components/schemas/Pet"
            application/json:
              schema:
                $ref: "#/components/schemas/Pet"
        "400":
          description: Invalid ID supplied
        "404":
          description: Pet not found
    post:
      tags:
        - pet
      summary: Updates a pet in the store with form data
      description: ""
      operationId: updatePetWithForm
      parameters:
        - name: petId
          in: path
          description: ID of pet that needs to be updated
          required: true
          schema:
            type: integer
            format: int64
      requestBody:
        content:
          application/x-www-form-urlencoded:
            schema:
              type: object
              properties:
                name:
                  description: Updated name of the pet
                  type: string
                status:
                  description: Updated status of the pet
                  type: string
      responses:
        "405":
          description: Invalid input
    delete:
      tags:
        - pet
      summary: Deletes a pet
      description: ""
      operationId: deletePet
      parameters:
        - name: api_key
          in: header
          required: false
          schema:
            type: string
        - name: petId
          in: path
          description: Pet id to delete
          required: true
          schema:
            type: integer
            format: int64
      responses:
        "400":
          description: Invalid ID supplied
        "404":
          description: Pet not found
  /store/inventory:
    get:
      tags:
        - store
      summary: Returns pet inventories by status
      description: Returns a map of status codes to quantities
      operationId: getInventory
      responses:
        "200":
          description: successful operation
          content:
            application/json:
              schema:
                type: object
                additionalProperties:
                  type: integer
                  format: int32
  /store/order:
    post:
      tags:
        - store
      summary: Place an order for a pet
      description: ""
      operationId: placeOrder
      requestBody:
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/Order"
        description: order placed for purchasing the pet
        required: true
      responses:
        "200":
          description: successful operation
          content:
            application/xml:
              schema:
                $ref: "#/components/schemas/Order"
            application/json:
              schema:
                $ref: "#/components/schemas/Order"
        "400":
          description: Invalid Order
  "/store/order/{orderId}":
    get:
      tags:
        - store
      summary: Find purchase order by ID
      description: For valid response try integer IDs with value >= 1 and <= 10. Other
        values will generated exceptions
      operationId: getOrderById
      parameters:
        - name: orderId
          in: path
          description: ID of pet that needs to be fetched
          required: true
          schema:
            type: integer
            format: int64
            minimum: 1
            maximum: 10
      responses:
        "200":
          description: successful operation
          content:
            application/xml:
              schema:
                $ref: "#/components/schemas/Order"
            application/json:
              schema:
                $ref: "#/components/schemas/Order"
        "400":
          description: Invalid ID supplied
        "404":
          description: Order not found
    delete:
      tags:
        - store
      summary: Delete purchase order by ID
      description: For valid response try integer IDs with positive integer value. Negative
        or non-integer values will generate API errors
      operationId: deleteOrder
      parameters:
        - name: orderId
          in: path
          description: ID of the order that needs to be deleted
          required: true
          schema:
            type: integer
            format: int64
            minimum: 1
      responses:
        "400":
          description: Invalid ID supplied
        "404":
          description: Order not found
  /user:
    post:
      tags:
        - user
      summary: Create user
      description: This can only be done by the logged in user.
      operationId: createUser
      requestBody:
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/User"
        description: Created user object
        required: true
      responses:
        default:
          description: successful operation
  /user/createWithArray:
    post:
      tags:
        - user
      summary: Creates list of users with given input array
      description: ""
      operationId: createUsersWithArrayInput
      requestBody:
        $ref: "#/components/requestBodies/UserArray"
      responses:
        default:
          description: successful operation
  /user/createWithList:
    post:
      tags:
        - user
      summary: Creates list of users with given input array
      description: ""
      operationId: createUsersWithListInput
      requestBody:
        $ref: "#/components/requestBodies/UserArray"
      responses:
        default:
          description: successful operation
  /user/login:
    get:
      tags:
        - user
      summary: Logs user into the system
      description: ""
      operationId: loginUser
      parameters:
        - name: username
          in: query
          description: The user name for login
          required: true
          schema:
            type: string
        - name: password
          in: query
          description: The password for login in clear text
          required: true
          schema:
            type: string
      responses:
        "200":
          description: successful operation
          headers:
            X-Rate-Limit:
              description: calls per hour allowed by the user
              schema:
                type: integer
                format: int32
            X-Expires-After:
              description: date in UTC when token expires
              schema:
                type: string
                format: date-time
          content:
            application/xml:
              schema:
                type: string
            application/json:
              schema:
                type: string
        "400":
          description: Invalid username/password supplied
  /user/logout:
    get:
      tags:
        - user
      summary: Logs out current logged in user session
      description: ""
      operationId: logoutUser
      responses:
        default:
          description: successful operation
  "/user/{username}":
    get:
      tags:
        - user
      summary: Get user by user name
      description: ""
      operationId: getUserByName
      parameters:
        - name: username
          in: path
          description: "The name that needs to be fetched. Use user1 for testing. "
          required: true
          schema:
            type: string
      responses:
        "200":
          description: successful operation
          content:
            application/xml:
              schema:
                $ref: "#/components/schemas/User"
            application/json:
              schema:
                $ref: "#/components/schemas/User"
        "400":
          description: Invalid username supplied
        "404":
          description: User not found
    put:
      tags:
        - user
      summary: Updated user
      description: This can only be done by the logged in user.
      operationId: updateUser
      parameters:
        - name: username
          in: path
          description: name that need to be updated
          required: true
          schema:
            type: string
      requestBody:
        content:
          application/json:
            schema:
              $ref: "#/components/schemas/User"
        description: Updated user object
        required: true
      responses:
        "400":
          description: Invalid user supplied
        "404":
          description: User not found
    delete:
      tags:
        - user
      summary: Delete user
      description: This can only be done by the logged in user.
      operationId: deleteUser
      parameters:
        - name: username
          in: path
          description: The name that needs to be deleted
          required: true
          schema:
            type: string
      responses:
        "400":
          description: Invalid username supplied
        "404":
          description: User not found
externalDocs:
  description: Find out more about Swagger
  url: http://swagger.io
servers:
  - url: 'https://{environment}.{host}/adsjfkklj'
    variables:
      environment:
        default: api
        enum:
          - api         # Production server
          - api.dev     # Development server
          - api.staging # Staging server
      host:
        default: swagger.ui    # Production server
    
components:
  requestBodies:
    UserArray:
      content:
        application/json:
          schema:
            type: array
            items:
              $ref: "#/components/schemas/User"
      description: List of user object
      required: true
    Pet:
      content:
        application/json:
          schema:
            $ref: "#/components/schemas/Pet"
        application/xml:
          schema:
            $ref: "#/components/schemas/Pet"
      description: Pet object that needs to be added to the store
      required: true
  schemas:
    Order:
      type: object
      properties:
        id:
          type: integer
          format: int64
        petId:
          type: integer
          format: int64
        quantity:
          type: integer
          format: int32
        shipDate:
          type: string
          format: date-time
        status:
          type: string
          description: Order Status
          enum:
            - placed
            - approved
            - delivered
        complete:
          type: boolean
          default: false
      xml:
        name: Order
    User:
      type: object
      properties:
        id:
          type: integer
          format: int64
        username:
          type: string
        firstName:
          type: string
        lastName:
          type: string
        email:
          type: string
        password:
          type: string
        phone:
          type: string
        userStatus:
          type: integer
          format: int32
          description: User Status
      xml:
        name: User
    Category:
      type: object
      properties:
        id:
          type: integer
          format: int64
        name:
          type: string
      xml:
        name: Category
    Tag:
      type: object
      properties:
        id:
          type: integer
          format: int64
        name:
          type: string
      xml:
        name: Tag
    Pet:
      type: object
      required:
        - name
        - photoUrls
      properties:
        id:
          type: integer
          format: int64
        category:
          $ref: "#/components/schemas/Category"
        name:
          type: string
          example: doggie
        photoUrls:
          type: array
          xml:
            name: photoUrl
            wrapped: true
          items:
            type: string
        tags:
          type: array
          xml:
            name: tag
            wrapped: true
          items:
            $ref: "#/components/schemas/Tag"
        status:
          type: string
          description: pet status in the store
          enum:
            - available
            - pending
            - sold
      xml:
        name: Pet
    ApiResponse:
      type: object
      properties:
        code:
          type: integer
          format: int32
        type:
          type: string
        message:
          type: string
  securitySchemes:
    basicAuth:     # <-- arbitrary name for the security scheme
      type: http
      scheme: basic
    bearer:
      type: http
      scheme: bearer
security:
  - basicAuth: []  
  - bearer: []

@smridge
Copy link
Contributor Author

smridge commented Sep 6, 2021

@develohpanda this is much better with everything in the .less file!

@reynolek
Copy link
Contributor

reynolek commented Sep 7, 2021

Ill pull this down tomorrow and test -- I think I saw material theme as another one that had issues, probably hyper too? Thanks for porting it deeper, helping all the themes as best we can without extra work 👍

Copy link
Contributor

@reynolek reynolek left a comment

Choose a reason for hiding this comment

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

Pulled down, and LGTM

@smridge
Copy link
Contributor Author

smridge commented Sep 7, 2021

Thank you @develohpanda & @reynolek for reworking and testing! Excited for this!

@develohpanda
Copy link
Contributor

@smridge thanks a tonne for figuring out what things needed to be changed, I was leaning on your initial fix a lot! 🙌🏽

I tried to do the same with the select, but it seems swagger-ui uses a custom icon for the dropdown to be able to position it further from the edge, but that then disconnects it from the font color. Hopefully that's the next thing to fix but it was getting too involved when I attempted.

@develohpanda develohpanda enabled auto-merge (squash) September 8, 2021 03:29
@develohpanda develohpanda merged commit 68f1bea into Kong:develop Sep 8, 2021
@reynolek
Copy link
Contributor

reynolek commented Sep 8, 2021

@smridge Congrats on your first contribution! Don't forgot to submit for your contributor tshirt (details in CONTRIBUTING.md)

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