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

Using a standard prefix for the CSS variables defined by this theme #305

Closed
jorisvandenbossche opened this issue Jan 22, 2021 · 4 comments · Fixed by #312
Closed

Using a standard prefix for the CSS variables defined by this theme #305

jorisvandenbossche opened this issue Jan 22, 2021 · 4 comments · Fixed by #312

Comments

@jorisvandenbossche
Copy link
Member

From @bollwyvl in #273:

I still really recommend doing a CSS variable prefix, e.g. --pst-

(I think you mentioned that in the original PR that added the CSS variables as well, but at that point I didn't get what you meant ;))

That indeed sounds like a good idea (and good practice) to do.

@choldgraf
Copy link
Collaborator

100% - it would be great if we did this after the next release, because it'll take downstreams a while to re-adjust any CSS they might be over-riding

@jorisvandenbossche
Copy link
Member Author

I don't fully understand the "after the next release" part. I was actually thinking: we need to do this before doing a release, because otherwise renaming those variables gives a breaking change for those who start to use them?

@jorisvandenbossche
Copy link
Member Author

Ah, or maybe you were thinking about prefixing CSS classes we use in the templates / CSS? That's indeed also something we should clean-up, rationalize and standardize as well, for sure! (-> #37) But so here I was specifically talking about the variables ("properties"), which are new in master, and we can still change without breaking users (except if you were using master).

Now, for the classes, when we would rename them, I am not fully sure what the best workflow would be to change them that is easiest for downstream projects. Probably just a hard break and clear release notes / docs about it?

@choldgraf
Copy link
Collaborator

ahhh yes - you are correct, I was misunderstanding you...I meant classes :-)

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

Successfully merging a pull request may close this issue.

2 participants