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

Convert the various colors in web/viewer.css to CSS variables #11572

Closed
timvandermeij opened this issue Feb 6, 2020 · 6 comments
Closed

Convert the various colors in web/viewer.css to CSS variables #11572

timvandermeij opened this issue Feb 6, 2020 · 6 comments
Labels

Comments

@timvandermeij
Copy link
Contributor

To make re-skinning the viewer easier, the various repeated colors in web/viewer.css should be converted to CSS variables. This way they should be easier to tweak for third-party deployments of PDF.js.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Feb 6, 2020

Before we start converting everything but the kitchen sink to use CSS variables, we may want to avoid adding "too many" until the performance impact (in Firefox) of doing so is better understood; based on stumbling upon https://groups.google.com/d/msg/firefox-dev/wQKKyH4vXks/iFmRdHBcEwAJ and https://bugzilla.mozilla.org/show_bug.cgi?id=1561001#c4 yesterday.

Edit: I obviously don't know exactly how many is too many, but we probably want to avoid adding hundreds of CSS variables.
Since there's already issues tracking the modernization/clean-up of the viewer UI, it'd probably make the most sense to introduce CSS variables as part of these improvements (which really need to happen iteratively, since changing the entire UI in one big commit is essentially impossible to test/review).

@rohanharikr
Copy link

Would like to work on this!

@Snuffleupagus
Copy link
Collaborator

Would like to work on this!

As already mentioned in #11572 (comment), this would be best done as part of other CSS modernization/improvement work:

Since there's already issues tracking the modernization/clean-up of the viewer UI, it'd probably make the most sense to introduce CSS variables as part of these improvements (which really need to happen iteratively, since changing the entire UI in one big commit is essentially impossible to test/review).

@rohanharikr
Copy link

@Snuffleupagus Got it!

@mitchell-frost
Copy link

I am new to mozilla. Can I work on this?

@timvandermeij
Copy link
Contributor Author

Fixed by #11077.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants