-
Notifications
You must be signed in to change notification settings - Fork 113
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(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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a space between .page
and .loadingIcon
? It's a child element, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have other .page .loadingIcon
styles a few lines up. Can we move these rules into the nested rules?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.