vlaaad / reveal

Read Eval Visualize Loop for Clojure
https://vlaaad.github.io/reveal/
MIT License
600 stars 21 forks source link

Refactored color handling in style.clj, added option to load color scheme via properties #11

Closed mvarela closed 4 years ago

mvarela commented 4 years ago

Hi Val! First of all, I wanted to thank you for putting reveal out there, it's really a great tool :) I had a look at the style code, and thought it might be good to factor out the colors into their own little structure, and make that user-replaceable via your prefs property. Here's a few changes that enable that. Basically, if the :color-scheme-file property is set (and it's a valid URL, we load its contents, and deep-merge it into a base colormap.

The base colormap looks like this:

(def colormap-base {:light {:scroll-bar-color                    "#0006"
                            :scroll-bar-background-color         "#eee3"
                            :scroll-bar-background-pressed-color "#fff6"
                            :inactive-scroll-bar-color           "#3333"
                            :selection-color                     "#64e3fc"
                            :unfocused-selection-color           "#e3e3e3"
                            :popup-color                         "#eee"
                            :search-color                        "#32383d"
                            :util                                "#999"
                            :symbol                              "#424b51"
                            :object                              "#c46003"
                            :string                              "018480"
                            :error                               "#f15856"
                            :success                             "#2a40ea"
                            :scalar                              "#2a40ea"
                            :keyword                             "#972aea"
                            :unfocused-background-color          "#f2f2f2"
                            :background-color                    "#fafafa"
                            }
                    :dark {:scroll-bar-color                     "#fff6"
                           :inactive-scroll-bar-color            "#eee3"
                           :scroll-bar-background-color          "#eee3"
                           :scroll-bar-background-pressed-color  "#fff6"
                           :selection-color                      "#005dd1"
                           :unfocused-selection-color            "#555"
                           :popup-color                          "#3b3b44"
                           :search-color                         "#aec1d0"
                           :util                                 "#888"
                           :symbol                               "#aec1d0"
                           :object                               "#f7b940"
                           :string                               "#22aeaa"
                           :error                                "#f15856"
                           :success                              "#64aa22"
                           :scalar                               "#649fe9"
                           :keyword                              "#ab6cf7"
                           :unfocused-background-color           "#333"
                           :background-color                     "#232325"
                           }})

It provides a source for the colors you use throughout style.clj.

I tried it with a theme I use for all my stuff, and it seems to work ok (I still need to make adjustments to the element-color pairs, but the code does what it's supposed to, at least).

If you save the following in, say, ~/suburn-reveal.edn, you should be able to see it in action by adding the JVM options to your the alias you're using:

{:dark {:scroll-bar-color                     "#383339"
        :inactive-scroll-bar-color            "#433844"
        :scroll-bar-background-color          "#484369"
        :scroll-bar-background-pressed-color  "#486399"
        :selection-color                      "#eead0e"
        :unfocused-selection-color            "#575684"
        :popup-color                          "#373664"
        :search-color                         "#888389"
        :util                                 "#7f9f7f"
        :symbol                               "#f0dfaf"
        :object                               "#dfaf8f"
        :string                               "#dca3a3"
        :error                                "#9c6363"
        :success                              "#5f7f5f"
        :scalar                               "#b48ead"
        :keyword                              "#8786b4"
        :unfocused-background-color           "#433844"
        :background-color                     "#484349"}}
  :jvm-opts   ["-Dvlaaad.reveal.prefs={:color-scheme-file,\"file:///home/vlaaad/sunburn-reveal.edn\"}"]

Let me know what you think :) Cheers, Martín

vlaaad commented 4 years ago

Hi @mvarela, many thanks for your contribution! I like both your ideas — loading prefs from an external source and custom themes, and in fact, I considered both before and I want to implement them in some way or another, but right now I haven't decided on any of those.

About styling: the colors accreted organically as I was developing Reveal and I would like to organize and categorize them better before making them a public API. Changes are probably coming to those constants since I want to change how result tabs look and feel. I'm also not happy with the fact that suburn-reveal.edn sets a :dark theme, I would prefer if :theme preference was either :light, :dark or a map with custom colors.

About loading from a file: I would prefer if the whole vlaaad.reveal.prefs could be a path to a file instead of a map literal. That will help with other issues people are having (like being unable to specify system font names that have spaces in their names). That will require some considerations with regard to how we check if the property value is a map literal or file path. Probably looking at a few first characters. Another point of concern there is whether I should allow URIs (so both http://... and file://... work) and whether in the case of file path I should also support ~ as a shortcut for home directory, which is not a baked-in thing Java.

With that in mind, I decided to not accept this PR because I don't have solutions for these problems right now, and I don't think this PR solves these problems, sorry. I'll be very grateful if you could give it a thought and share your findings in problem areas of styling/loading prefs— I'd love to have custom theming!

mvarela commented 4 years ago

Hi Vlad, no worries about not merging, this was more a suggestion rather than anything else. A couple of comments:

Regarding the use of a config file, I think it would be much, much better than setting properties. Maybe doing something like clj does, where you can have a user-level config in your home dir, which can be overriden by one in the project folder (or in the classpath, anyway), and finally setting the properties via -D.... I think it would be much more ergonomic than the current solution, and it should be simple to implement, too.

Anyway, if I can somehow help with this, let me know :)

mvarela commented 4 years ago

Just another thought, regarding the config file property, rather than trying to figure out from the string whether it's a URL or a map, you could have have a vlaaad.reveal.configfile property to override the "default search path" for the config, and keep the current property with the preferences for backward compatibility, if needed (merging its contents over the deafult config loaded from the file, or just overwriting them, if the vlaad.reveal.prefs property is available.