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

Adds dark mode to entire dashboard #467

Merged
merged 33 commits into from
Dec 16, 2020
Merged

Conversation

Vigasaurus
Copy link
Contributor

@Vigasaurus Vigasaurus commented Dec 13, 2020

Changes

Adds dark mode user option and system setting adherence across the entirety of the dashboard and otherwise authenticated user flows.

I do have this currently running at https://dev.vigneshjoglekar.com/plausible.io - but this is my personal VPS for all staging+my VS Code server so it could easily be down at random times. The benefit here is that I've got the real plausible demo data being pulled in, to make testing easier. (Feel free to let me know if its ever down and I can get it spun back up).
The above does also have a user setup if you wanted to check out other pages - user: test@test.com, pw: passwd

Definitely do let me know if there are any issues or missing elements etc - I'd be happy to get them resolved!

Fixes #115

Tests

  • Automated tests have been added
    My Elixir is weak at best, I'm mostly unsure how/where exactly I'd add tests for my changes. I imagine it'd only be for the extra column in the user table, and its modification - but I'm unsure. I did also have to add a migration.

Changelog

  • Entry has been added to changelog

Documentation

  • Docs have been updated
    Again, I'm unsure if this needs any documentation change - besides maybe the contributors section 😉

Screenshots

image
image
image
image

@jnessier
Copy link

I like the dark gray styles more, but your blueish style looks cool too. Good job!

@ukutaht
Copy link
Contributor

ukutaht commented Dec 14, 2020

Looks great

@Vigasaurus
Copy link
Contributor Author

Thanks! To be fair, most of my color choices revolve around the mockup that NuroDev made.

@ukutaht
Copy link
Contributor

ukutaht commented Dec 14, 2020

@Vigasaurus ready for review?

@Vigasaurus
Copy link
Contributor Author

Vigasaurus commented Dec 14, 2020

@ukutaht Yep 👍

Copy link
Contributor

@ukutaht ukutaht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I'm eager to merge. I have a few comments that I'd like to hash out, let's discuss further in replies to those individual comments.

EDIT: sorry about the merge conflicts. I am not working on any UI code, probably for the rest of the week. So I can promise no more conflicts if you do fix them at this point.

assets/css/app.css Outdated Show resolved Hide resolved
assets/js/dashboard/stats/countries.js Outdated Show resolved Hide resolved
assets/js/dashboard/stats/countries.js Show resolved Hide resolved
lib/plausible_web/templates/auth/register_form.html.eex Outdated Show resolved Hide resolved
lib/plausible_web/templates/layout/app.html.eex Outdated Show resolved Hide resolved
lib/plausible/themes.ex Outdated Show resolved Hide resolved
@Vigasaurus
Copy link
Contributor Author

👍 all comments make sense, I'll get em resolved tomorrow (in like 8-10 hours probably) - and I've got the new stuff on master almost done anyways.

@ukutaht
Copy link
Contributor

ukutaht commented Dec 15, 2020

+1 all comments make sense, I'll get em resolved tomorrow (in like 8-10 hours probably) - and I've got the new stuff on master almost done anyways.

Of course, no pressure. If you don't finish it by tomorrow that's no problem. We are incredibly thankful for your contributions!

@Vigasaurus
Copy link
Contributor Author

This should be all good to go - I'll likely do another once-over to see if I missed anything with the added stuff/conflic resolution, but it should be ready :D

@ukutaht
Copy link
Contributor

ukutaht commented Dec 16, 2020

Remove 'CountriesWrapper' from here.

And are you using 4 spaces? The rest of the project is 2 spaces, let's try to keep it consistent.

Updates look good. I will merge once formatting issues are solved

@Vigasaurus
Copy link
Contributor Author

Yeah, I was using tabs, I would've assumed that VS Code would've picked the same indent style as the rest of the repo 🤷‍♂️ whoops.
Both issues fixed.

@ukutaht
Copy link
Contributor

ukutaht commented Dec 16, 2020

Thank you so much @Vigasaurus! This is a big job done well.

I will deploy this later today 🚀🚀🚀🚀

@ukutaht ukutaht merged commit 425975e into plausible:master Dec 16, 2020
@metmarkosaric
Copy link
Contributor

nice!! looking forward to announcing this to the community and putting @Vigasaurus in our hall of fame list of contributors!

oliver-kriska pushed a commit to payout-one/analytics that referenced this pull request Dec 17, 2020
* Adds New Dark Mode Assets

* Moves triangle for dropdown to a reasonable position

* Majority .eex dark implementation

* Fixes Logo Positioning

* Adds theme flag to user schema, uses it

* Uses correct variables for theme applicator script

* Minor missed theme changes/fallbacks

* Individual Component Support + Theme Context

* Sources Tab Support

This was a pain to test D:

* Partial Stats Sections Support

* More of stats modules supported

* Modal +table support

* Improves some Flatpickr in light theme, supports dark theme

* Fixes missed settings tab colors

* Finishes Devices module support

* Fixes bar graph colors

* Better colorizes maps module

* Undoes colorized bars

(they looked bad, on second thought)

* Fixes loading indicator

* Finishes conversions module

* Adds changelog entry

The PR number could be wrong, will double check

* Fixes missed header color

* Fixes naming of migration and removes static alter

* Does migration correctly

As I said, my Elixir is pretty weak heh

* Adds support for spike notifications setting

* Improves contrast and visibility for email settings

* Resolves @ukutaht's comments on plausible#467

* Fixes missing dark style

* Found one more missed dark element (shared links)

* Formatting fixes
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.

4 participants