Skip to content

Conversation

@vicchi
Copy link
Contributor

@vicchi vicchi commented Jul 19, 2019

Ensures that all background SVGs in the SCSS source are not fully or partially Base64 or URL encoded.

Description

This PR fixes some inconsistencies in the inline background SVG definitions in src/style/_form.scss.

We're using swagger-ui embedded in our main web site and use a clone of the main repo to enable us to apply styling overrides to the SCSS source as we compile our site's assets (as opposed to using the pre-built npm packages).

Our build workflow includes several postcss optimisations, which were borked on two SVG definitions.

The first is an SVG which is Base64 encoded (and which is also covered by a PR to the postcss-encode-background-svgs plugin). This PR replaces that SVG with an unencoded version, including stripping out extraneous tags from the application that originally generated the SVG.

The second is a partially URL encoded SVG which included invalid, non-printing characters (LF). This PR replaces the SVG with an unencoded version, again stripping out extraneous application generated tags as well as the LF.

Motivation and Context

As noted above, to override some styling in the generated UI, we're including the source SCSS in our site's generated CSS. The inconsistent SVG definitions cause issues with upstream CSS optimisations, including those from postcss and associated plugins.

How Has This Been Tested?

  • By running the repo test suites via npm run test and npm run e2e
  • Test environment: macOS 10.14.5 (Mojave), Selenium 3.141.59, all other dependency versions as per package.json; all tests pass.
  • Oh and this works perfectly in our (soon to be launched) API documentation too!

Screenshots (if appropriate):

Not applicable.

Checklist

My PR contains...

  • No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
  • Dependency changes (any modification to dependencies in package.json)
  • Bug fixes (non-breaking change which fixes an issue)
  • Improvements (misc. changes to existing features)
  • Features (non-breaking change which adds functionality)

My changes...

  • are breaking changes to a public API (config options, System API, major UI change, etc).
  • are breaking changes to a private API (Redux, component props, utility functions, etc.).
  • are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
  • are not breaking changes.

Documentation

  • My changes do not require a change to the project documentation.
  • My changes require a change to the project documentation.
  • If yes to above: I have updated the documentation accordingly.

Automated tests

  • My changes can not or do not need to be tested.
  • My changes can and should be tested by unit and/or integration tests.
  • If yes to above: I have added tests to cover my changes.
  • If yes to above: I have taken care to cover edge cases in my tests.
  • All new and existing tests passed.

vicchi and others added 9 commits July 19, 2019 08:34
… URL encoded SVGs with unencoded version, removing extraneous LF character
… URL encoded SVGs with unencoded version, removing extraneous LF character
… URL encoded SVGs with unencoded version, removing extraneous LF character
… URL encoded SVGs with unencoded version, removing extraneous LF character
@tim-lai tim-lai merged commit 48a0b46 into swagger-api:master Jun 11, 2020
@vicchi
Copy link
Contributor Author

vicchi commented Jun 11, 2020

@tim-lai Thanks!

mattyb678 pushed a commit to mattyb678/swagger-ui that referenced this pull request Jun 24, 2020
* Replace Base64 encoded SVGs with unencoded version. Replace partially URL encoded SVGs with unencoded version, removing extraneous LF character
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.

2 participants