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

[Feature Request/Discussion] Runtime "Theme Engine" aka advanced tinting #5844

Open
MTRNord opened this issue Dec 16, 2017 · 30 comments
Open

Comments

@MTRNord
Copy link
Contributor

MTRNord commented Dec 16, 2017

I asked yesterday in #riot-dev what the opinions about allowing to load css at runtime via a UI plus a Theme design UI or advanced tinting aka. allowing to modify different components by CSS values easily. (You can reread it here: https://matrix.to/#/!DdJkzRliezrwpNebLk:matrix.org/$15133678091603OtUhP:matrix.ffslfl.net )

This issue is more a Request/Tracking issue about what we already discussed and what the Community prefers.

Things we discussed/mentioned/talked about:

  • Similiar possibilities to what Telegram in their clients allows
  • Allowing to modify Components like Buttons independently from the Rest of the Theme
  • Allowing to save these Themes in a file format (Propably a css file is not wanted as you could abuse it. I am thinking about a XML file that gets parsed)
  • Having some UI for generating these Theme files
  • In general making custom Themes for non Devs easier. Especialy if someone uses the Desktop App ( https://matrix.to/#/!DdJkzRliezrwpNebLk:matrix.org/$15133685521608ayXMf:matrix.ffslfl.net )
@pafcu
Copy link
Contributor

pafcu commented Dec 16, 2017

I'm not saying it's not possible, but how could the CSS be abused, and would it be possible to prevent those things specifically?

@MTRNord
Copy link
Contributor Author

MTRNord commented Dec 16, 2017

@pafcu depending on how you load it you could do </style><script>something();</script> in a file and it gets loaded to DOM like this. But of course you might normalize it to strip these things anyway.

@pafcu
Copy link
Contributor

pafcu commented Dec 16, 2017

I'd just add the content of the CSS file as text to a script element. That automatically prevents that.

@MTRNord
Copy link
Contributor Author

MTRNord commented Dec 16, 2017

@pafcu well thats something I didn't know of before :) In that case a CSS would work of course

@pafcu
Copy link
Contributor

pafcu commented Dec 16, 2017

In any case, you could probably do a bit of damage with CSS anyway (think fake display names, hide encryption warnings, leak usage by loading external resources) so we need to consider how much we want to trust theme authors, and what we want to prevent / sanitize.

@MTRNord
Copy link
Contributor Author

MTRNord commented Dec 16, 2017

@pafcu I feel like that the color: value only should allow colors that aren't near the background color for example. Means to check their value if it is similiar or not

@MTRNord
Copy link
Contributor Author

MTRNord commented Dec 16, 2017

Also using custom svg might make problems with plain css files.

@pafcu
Copy link
Contributor

pafcu commented Dec 16, 2017

What problems would custom SVG cause?

@MTRNord
Copy link
Contributor Author

MTRNord commented Dec 16, 2017

@pafcu can you replace the svg files of the buttons with plain css? aren't thoose defined via html objects?

@pafcu
Copy link
Contributor

pafcu commented Dec 16, 2017

But that applies to most images, not just SVG (unless they are implemented using CSS bsckground, which causes other problems)

@lampholder
Copy link
Member

How far is Slack's impl from what you're looking for?

Slack supports a very limited custom styling, enumerating stylable components and allowing you to specify any colour value:
image

And it has no defense against a fully auberginetastic theme, though it is purely per-client (you share themes by users' copy+pastaing color value strings into their own client):
image

@MTRNord
Copy link
Contributor Author

MTRNord commented Dec 18, 2017

What Telegram does: Atleast on mobile:
https://telegram.org/blog/android-themes

Their desktop Client seems to support themes like the Mobile ones too

@turt2live
Copy link
Member

I'd be kinda surprised if Riot did actually restrict colors, as it'd probably get in the way more than normal or be too inaccurate (one person's definition of "close to" is going to differ from another's). If the user really wants everything to be hot pink, let them have it at their own risk.

I like slack's approach, and I've seen it quite often outside of chat as well. Riot could do with a similar number of options where you won't be able to change things like the encryption status icons.

@pafcu
Copy link
Contributor

pafcu commented Dec 18, 2017

I'm honestly not a huge fan of the Slack syntax for it, because just having a list like that much be terrible for backward compatibility. Perhaps e.g. Base64-encoded compressed JSON would be able to achieve something similar?

@MTRNord
Copy link
Contributor Author

MTRNord commented Dec 18, 2017

I agree with the UI of Slack but as @pafcu says not with the Syntax/format of data

@turt2live
Copy link
Member

Keeping it in an easily editable form makes sense to me. Base64 is probably going over the top, but JSON wouldn't be a bad idea. Realistically it should be something that someone can paste into the box and be able to edit it in the field - and know what they're changing.

@pafcu
Copy link
Contributor

pafcu commented Dec 18, 2017

I don't see why you'd want to be able to edit by hand when you have a perfectly fine UI to do it. What's the use case for editing it by hand outside an app that understands the format?

edit: To clarify, nothing would stop you from having a text field with the uncompressed JSON in Riot, which would enable plain-text editing. It seems to be that having a short "oneliner" to copy-paste around would be a lot nicer than to have a nice, pretty-printed 10 line JSON object that you send by Riot to your friends.

@turt2live
Copy link
Member

People may want to put the thing in version control, company documentation, or really anywhere that isn't Riot. It's relatively common for companies here to provide that information at least in internal documentation. It's more so that marketing and ops (or whichever team is deploying the thing) can keep things consistent, even if both departments go through a complete turnover.

@pafcu
Copy link
Contributor

pafcu commented Dec 18, 2017

Hmm, perhaps accept both? It seems like for your case it would anyway be much better to have themes in a separate file, rather than a copy-pastable string.

@MTRNord
Copy link
Contributor Author

MTRNord commented Dec 18, 2017

I agree with the file approach, Makes it simpler (and is what Telegram for example does)

@turt2live
Copy link
Member

I just don't see the benefit of having a "share with your friends" section if it's hard to share it with your friends.

Consider the following scenario:

  • Bob comes up with a theme that they think looks nice
  • Bob sends the theme to Alice
  • Alice likes it, but changes a couple values
  • Alice sends it back to Bob

If Bob can't see Alice's changes easily, there's no real point in that exchange at all, making it become one-sided (Alice tells Bob to change the colors instead of doing it themselves).

A file might work, but I really don't see a problem with a JSON string. It's not going to be that long.

@turt2live
Copy link
Member

turt2live commented Dec 18, 2017

Re-reading the original post does mean the JSON would be insanely long, as it implies being able to change every minuscule aspect of the app. From experience, this is a somewhat bad idea as it leads to confusion and very complex UIs. Slack's approach, where you're only able to edit 8 things, seems like a better way forward: it reduces the complexity of the UI, is approachable, and keeps the app looking at least somewhat reasonable.

@pafcu
Copy link
Contributor

pafcu commented Dec 18, 2017

I just don't see the benefit of having a "share with your friends" section if it's hard to share it with your friends.

I assumed that is why Slack has an easily copy-pasteable string, but it might of course be for some completely other reason.

@MTRNord
Copy link
Contributor Author

MTRNord commented Dec 18, 2017

@turt2live well you can just load the json into a file. That makes it again easier to have them in repos. :) just download the raw file and import it or maybe even easier (for a user) to just use the file link. (but harder to implement)

@turt2live
Copy link
Member

A file is a bit harder to deal with when it's embedded in internal documentation though. Really it ends up being how long this string is. If Riot allows the user to change every tiny aspect then I'd recommend a file. If it's only 8 colors, then I'd recommend it just be a string - the people who want it in repos can put it in a file if they want to.

@turt2live
Copy link
Member

As another data point, here's a fairly rough UI I ended up creating for my day job. It's a proof of concept for theming the application so that customers can re-sell the product.

image

@ara4n
Copy link
Member

ara4n commented Sep 17, 2019

I think we should provide a basic colour picker and font picker to let people easily reskin the app a bit for their personal preferences in terms of contrast, font legibility (particularly proportional v. monospace, and supporting Greek & Japanese etc to mitigate like #7949). Brownie points if we can do it per-room via granular settings so we can recover the old tinting functionality.

I do not think we should encourage people to apply freeform CSS overrides - if you're geeky enough to do that, you should use userstyle or greasemonkey or whatever the cool kids use these days. Maintaining custom CSS is always going to be fragile as we evolve the app, and we don't want to encourage folks to build fragile themes.

@ara4n ara4n added the P2 label Sep 17, 2019
@ghost
Copy link

ghost commented Sep 19, 2019

I'm more concerned with the loading of a custom font installed on the OS with riot tbh. For example on Discord and other chat clients on linux can be forced to load a custom font OS wide if a font.conf (.xml) is placed in ~/.config/fontconfig/fonts.conf matrix is electron like Discord but for some reason it ignores/overrides the font config file in the home folder.

@ara4n
Copy link
Member

ara4n commented Mar 3, 2020

Just in case anyone missed it, we do now have a runtime theme engine as per https://github.com/vector-im/riot-web/blob/develop/docs/theming.md. It needs to be exposed nicely, though. matrix-org/matrix-react-sdk#4148 could be one step in the right dir.

@MTRNord
Copy link
Contributor Author

MTRNord commented Mar 3, 2020

Thats awesome :D That is 60% of what I had in mind :) (I was more on the telegram route. But having that ability for hosters is a good starting point)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants