search

deepjyoti30 / startpage

A minimal starpage for Chrome and Firefox
MIT License
625 stars 85 forks source link

Make some background colors and text colors configurable through new `style` configuration object #38

Closed benjih closed 3 years ago

benjih commented 3 years ago

Hey @deepjyoti30, great work so far on this project.

I set it up locally and enjoyed the simplicity but quickly found an edge case that prompted me to make this change.

I wanted to use this startpage across multiple browsers and profiles due to division in work / personal / etc but found myself needing more of a larger visual indicator as to which browser / session I was looking at. The simplest indicator I could think of was changing some colors.

I know that this could have been possible by locally changing the CSS file, but that would have meant needing multiple copies locally on my HDD, each with different CSS files. In my usecase of different configurations of it running in parallel, it felt much easier for me to rely on the browser session's local storage.

Before implementing this, I did look at previous Issues and Pull Requests to see that there had been talk about implementing some kind of theming or styling without code change.

Let me know your thoughts. More than happy to make some minor changes if this is a feature you'd like but think some things should be implemented differently.

If this does get merged, I'm more than happy to update the configuration page in the wiki to include the changes I've made.

reesericci commented 3 years ago

doesn't look bad to me but I haven't tested it

benjih commented 3 years ago

Ah, forgot to mention. I manually tested it on Google Chrome with no problems. I will build and deploy a Firefox version and report back.

benjih commented 3 years ago

Just tested it on Firefox and seems to work perfectly.

benjih commented 3 years ago

Was just reviewing what I had done and was playing around with some different configurations and noticed that I had forgot to also style the line divider and the weather elements. I have just fixed that.

deepjyoti30 commented 3 years ago

@benjih Thanks for this. Yes, I have been meaning to add some kind of configuration for the colors. I will take a thorough look at the commits.

I would like to test this before going forward since it will be a huge move. Also need to make sure all edge cases are considered.

deepjyoti30 commented 3 years ago

@benjih @reesericci I have merged your PR as well as added some fixes. Consider trying out the changes!

Also you can go ahead and update the WiKi with the changes!