fix(pdf): override css missing asset #1505
Conversation
|
Could we add a short description about what issue this fixes with the PR? |
src/lib/viewers/doc/_docBase.scss
Outdated
| @@ -295,6 +295,17 @@ $thumbnail-sidebar-width: 191px; // Extra pixel to account for sidebar border | |||
| } | |||
| } | |||
|
|
|||
| //overwriting background image in pdf_viewer.css. Was causing 404's | |||
| /* stylelint-disable declaration-no-important */ | |||
| .pdfViewer .page.loadingIcon { | |||
There was a problem hiding this comment.
Should there be a space between .page and .loadingIcon? It's a child element, right?
There was a problem hiding this comment.
Believe it or not no. That's how pdf.js v3.6.7.2 has it. I can take out line 300 but I think we need the other one. Check this out https://github.com/mozilla/pdf.js/blob/v3.6.172/web/pdf_viewer.css#L162
src/lib/viewers/doc/_docBase.scss
Outdated
| background-image: none !important; | ||
| } | ||
|
|
||
| .pdfViewer .page.loadingIcon::after { |
There was a problem hiding this comment.
We have other .page .loadingIcon styles a few lines up. Can we move these rules into the nested rules?
package.json
Outdated
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "box-content-preview", | |||
| "version": "2.101.0", | |||
| "version": "2.100.0", | |||
There was a problem hiding this comment.
Is this intentional? Seems like it would cause problems.
| @@ -289,6 +289,12 @@ $thumbnail-sidebar-width: 191px; // Extra pixel to account for sidebar border | |||
| background: none; | |||
| } | |||
|
|
|||
| &.loadingIcon::after { | |||
There was a problem hiding this comment.
This should prevent the browser from calling out to the loading icon file, but I don't think it will fix our custom loading experience. We'll need a follow-up change and release for that.
Because of a change in a css rule in the latest version of pdf_viewer.css we were not overriding the loadingIcon style's background image which caused the CDN to record a large number of 404's as a result.