Skip to content

Add light theme #98

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.

Sign up for GitHub

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

Jump to bottom
Closed
adtac opened this issue Apr 13, 2019 · 4 comments · Fixed by #120
Closed

Add light theme #98

adtac opened this issue Apr 13, 2019 · 4 comments · Fixed by #120
Assignees
@adtac
Labels
feature request New feature request help wanted Extra attention is needed

Comments

@adtac
Copy link
Contributor

adtac commented Apr 13, 2019 •
edited
Loading

General information

  • Operating system + version: Arch Linux
  • Browser + version: Firefox Nightly (2019-04-13)
  • Information about the host app:
    • How did you install it? browserpass from [community]
    • If installed an official release, put a version ($ browserpass --version): 3.0.6
  • Information about the browser extension:
    • How did you install it? Firefox Add-ons store
    • Browserpass extension version as reported by your browser: 3.0.9

Browserpass v2 used to have a light theme and it was great. But it looks like v3 only supports a dark theme. I know that dark themes are more popular with developers (I, too, used to use dark themes), but a small minority of us prefer light themes. FWIW, dark text on light backgrounds is supposed to be more legible and readable to human eyes if the room you're in is well lit (which is the case for me).

As a worst case scenario, allow adding custom override CSS like available in the Tab Center Redux extension. But I'd much rather prefer a maintained light theme because div IDs and classes may change across releases, making custom CSS difficult to maintain.
@maximbaz
Copy link
Member

Initially I had my reservations about supporting both themes (just because of the increased maintenance cost), but I believe it's a valid request and if we implement it right so that increased cost is minimal, I won't oppose a PR.

I propose to move everything color-related out of popup.less into popup-dark.less, and create a new popup-light.less file. Hopefully most of the styles will remain in a shared file, and the theme-related files will be minimal. Then add a class theme-dark or theme-light on <body> tag based on the current settings (keep dark theme a default), and in popup-*.less wrap everything in .theme-dark { ... } or .theme-light { ... } accordingly.

I don't think I'll be working on a light theme myself, so a PR is very welcome 🙂

If you would like to see a light theme, please upvote the issue, once someone proposes a PR I'll provide you with a test build and ping you for feedback.

P.S. Since you are on Arch, I'll use this opportunity to recommend you to replace the addon from Web store with browserpass-firefox from [community]. I've just realized that this repo doesn't even mention this package in README, will fix in a moment.

@maximbaz maximbaz added feature request New feature request help wanted Extra attention is needed labels Apr 13, 2019
@adtac
Copy link
Contributor Author

adtac commented Apr 14, 2019 •
edited
Loading

Thanks, I've switched to the [community] version now.

And I'll try to submit a PR later this week. I can't maintain it, but I'm willing to do the initial work required.

@erayd
Copy link
Collaborator

erayd commented Apr 14, 2019

@adtac Thank you - will keep an eye out for your PR.

Once it's merged, we will maintain it as part of the main codebase.

@erayd erayd removed the help wanted Extra attention is needed label Apr 14, 2019
@maximbaz maximbaz added the help wanted Extra attention is needed label Apr 14, 2019
@maximbaz maximbaz mentioned this issue Apr 15, 2019
Closed
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 16, 2019
@adtac
Add light theme
cdb2639 
Closes browserpass#98
@adtac adtac mentioned this issue Apr 16, 2019
Merged
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 16, 2019
@adtac
Add light theme
a7aeeae 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 16, 2019
@adtac
Add light theme
ebd4104 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 16, 2019
@adtac
Add light theme
c6af93b 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 16, 2019
@adtac
Add light theme
5ebde36 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 16, 2019
@adtac
popup: Add light theme
50d64eb 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 16, 2019
@adtac
popup: Add light theme
9ad0825 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 16, 2019
@adtac
popup: Add light theme
d434309 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 16, 2019
@adtac
popup: Add light theme
9d2dc33 
Closes browserpass#98
@maximbaz
Copy link
Member

Please provide your feedback in #120, I'm happy to create test builds for you if necessary.

adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 17, 2019
@adtac
popup: Add light theme
2d2f4bc 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue Apr 18, 2019
@adtac
popup: Add light theme
2270e55 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue May 2, 2019
@adtac
popup: Add light theme
cf6b089 
Closes browserpass#98
adtac added a commit to adtac/browserpass-extension that referenced this issue May 2, 2019
@adtac
popup: Add light theme
25bc688 
Closes browserpass#98
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adtac adtac

Labels
feature request New feature request help wanted Extra attention is needed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add light theme adtac/browserpass-extension
3 participants
@erayd @maximbaz @adtac