Use css custom properties for colors

[Mavrin]

There is POC of theme api. It can be merged safely. I suppose, it is not should affect current behavior for end user.

What I did:

• I added stylelint rule which check that we use css custom properties for all color usage. Thank you @lahmatiy for help with plugin and csstree.
• I added postcss-custom-properties plugin for fallback for browsers which doesn't support css custom properties
• I replaced all static color on css custom properties

How does it work?

You can uncomment in index.ejs

        // __colors__: {
        //   "--color0": "red",
        // },

and notice that comment change color

I use __colors__ because it not final implementation.

What is next?

• We need reduce count of colors. Now we have 49 colors. I think, we can reduce that and make derived colors, for example

  const bgPanel = color(colorBody)
    .mix(color(colorContent), 0.025)
    .toString();

• We should define how api will looks for user who embed remark42. I think, it will be something like this:

var remark_config = {
        site_id: 'remark',
        host: window.location.origin,
        url: 'https://remark42.com/demo/',
        components: ['embed', 'counter'],
        theme: { // if it is string fallback on old behavior
           mode: "dark",
           font: {
               family: "Helvetica, Arial, sans-serif"
           },
           colors: {
              color0: "green",
              ...
           }
        }
      };

Comments are welcome.

[paskal]

I am not able to review this, but from the description, it seems like there are multiple changes included in single PR and generally I would advise to move them to separate PRs to simplify the reviewer job.

[Mavrin]

@paskal Of course it can be split, but it doesn't make sense most of changed files is replace by full search and if I split this request on smaller, it can't help with review this request.

[paskal]

It's up to other reviewers, but in such a situation I personally would prefer having two more PRs with 1 file changes in each and this with 63 files, rather than one with 65. Mixing multiple intentions in single PR usually makes my life as a reviewer harder, and it seems that this PR is a mix of custom properties added plus some style and linter improvements that are outside of that scope.

[Mavrin]

Linter is not outside of that scope. You can consider it like tests, it verifies that I don't miss any css files witch contains css colors.

[umputun]

this is an interesting idea. The question I have – is it limited to colors only? From your example, it looks like fonts will be supported as well, correct me if wrong. What else (if any) can we support in the same way?

[Mavrin]

Yes,fonts also will be supported. I am going add it in next pull request. Also we can add support padding, font-size, but it isn't easy. My first goal is colors and font-family.

[umputun]

Another question - wouldn't be uploading a regular css more generic and easier to implement? I mean adding some API method allowing admins to upload css file with whatever user need? The method (smth like POST /css?site-id=blah) will save css overwrites per site-id and will be paired with GET /css?site-id=blah to serve it. We can add server-side logic to this GET, like returning default (empty) css in case if the admin hasn't posted any. At the same time, we could apply some modifications to the response, like enforcing "!important" suffixes (if needed, just an example), verify CSS validity, and so on.

Am I missing something and this is not a good idea? To me, it sounds like almost no work on the UI side (well, maybe minimize and consolidate styles to make it more friendly for "rewrites") and mostly trivial change for the backend. I have seen something similar in both matermost and rocket chat, both allow to enter css manually.

[Mavrin]

Add support custom css files , it can be next step. But it is not easy support css selector between remark42 version, and it will be hard admin user figure out what to need change in css for new released version.
Look how it do slack

[umputun]

But it is not easy support css selector between remark42 version, and it will be hard admin user figure out what to need change in css for new released version.

Theoretically this "migration issue" can be mitigated on the server-side. I.e. we provide several basic selectors and maintain proper mapping. Not sure I like this approach as it leaks parts of frontend logic to the backend side.

Or even better - we define a list of "exposable selectors" and never change them. This will definitely make ui development less flexible as those selectors going to be a part of "public API".

Either way - up to you, just an idea I wanted to share.

[akellbl4]

I support the approach with adding CSS files.
We can make two options of customization "simple" and "advanced".
In first it is just a definition of custom properties. We can provide an example with a list of colors and demo page with few variants of color customization, also it can be an "online editor" for CSS.
The second way is an attempt to fix class names, I think it is possible that we will take care of it. For example, we can start from small UI elements like buttons and inputs and then go forward with bigger elements.
Yes, of course, some time selectors can be changed but I see only two reasons:

1. Refactoring - we can change complex parts of the interface and it will corrupt parts with layout stylization (I mean when a user adds styles to layout blocks). I think we should make rules of style customization and try to keep compatibility with previous versions.
2. New features - it is definitely a migration issue.

It is a long way to make it good enough for using and I think we can start to provide this feature like alfa or beta, because our "API" can change before we make final version.

P.S. Example with Slack is good but it is a standalone application and theming it is a minor feature for them. But for us, it can be a major feature because I think any comment system provides it.

[akellbl4]

LGTM from code point of view.

[Mavrin]

@umputun could you merge it?

[ColasNahaboo]

I wws going to comment (prompted from #5 (comment) ) but I would only repeat what umputun has said above:

• Just support adding CSS files.
• define a set of documented selectors.
• if you must change them, document the changes in the release notes.