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

Allow applying custom themes. #63

Closed
Apollo3zehn opened this issue Feb 17, 2019 · 5 comments
Closed

Allow applying custom themes. #63

Apollo3zehn opened this issue Feb 17, 2019 · 5 comments

Comments

@Apollo3zehn
Copy link

It would be great if this extension would allow applying custom themes as suggested in this post. My aim is to have a similar appearance in vscode like the graph I created in the online editor.

If I understand these lines correctly, the configuration is loaded but unfortunately the fields "theme" and "themeCSS" are overriden. To keep flexibility I would like to suggest to only override these settings if they are not present in the previously loaded config.

@vstirbu
Copy link
Collaborator

vstirbu commented Feb 18, 2019

Seems like a sensible approach to apply the theme and themeCSS if they are available in the loaded configuration. In this case the user would be fully responsible for the end result (e.g. rendering). Would that be ok?

@Apollo3zehn
Copy link
Author

If the new behavior is well documented, I think it is a feasable approach.

@vstirbu
Copy link
Collaborator

vstirbu commented Feb 18, 2019

Also one option to use @uifabric/merge-styles and merge the loaded styles into the defaults (e.g. the current ones). With this approach, at least, the customisation starts from a sane state.

@Apollo3zehn
Copy link
Author

Apollo3zehn commented Feb 18, 2019

I had problems overriding defaults, even if I use !important. So if in a config the theme is set to "theme": null (or any other custom theme), there should be no defaults. If - on the other hand - there is no "theme:" but only "themeCSS": defined, then I think @uifabric/merge-styles is a good idea.

@vstirbu
Copy link
Collaborator

vstirbu commented Feb 19, 2019

Ok! Lets leave the user in full control for now. If there is demand to do the merge, it can be done later with merge-styles

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

No branches or pull requests

2 participants