midzer/tobii

Custom Properties Scoping

jakobhaerter opened this issue · 8 comments

jakobhaerter commented

Naming of tobii's custom properties interferes with local custom properties due to generalized naming like --button-background or --button-color, set on :root level. A possible solution would be to scope variables inside tobii's parent node or to prefix everything, e.g. --tb-button-background.

Thanks!

midzer commented

Good idea, @jakobhaerter and thanks for filling!

As we have two possible solutions, which one would you prefer?

Prefixing those properties with --tobii-button-background might be the easier one. Are there any drawbacks?

  • 👍2
viliusle commented

There is very very low risk breaking somebody code, but fixing it would give much more benefits.

midzer commented

@jakobhaerter @MoritzLost you want to help out and become a first time contributor for tobii?

MoritzLost commented

@midzer @jakobhaerter @viliusle Sure, I've opened a PR for this update here: #99

The prefix approach is simpler. Scoping the variables to .tobii would be a good idea as well, but there's two problems:

  • Tobii also uses custom properties for some other classes, like tobii-zoom, so the properties would need to be added to all of those.
  • Changing the scope to those classes would result in a BC break, because then they would overwrite custom properties defined by users in the :root scope.

I think adding prefixes is completely sufficient. The PR adds those prefixes but prefers the old custom property names, if they still exist: var(--lightbox-background, var(--tobii-lightbox-background));

This way, the update is backwards compatible, so it can be a minor release. The fallback can then be dropped for the next major release.

By the way, I've noticed the project lacks a changelog, so I've added one in the PR. The changelog is a good place to include update instructions and deprecation notices like this one. @midzer Let me know if you want me to remove the changelog from the PR and/or add the update instructions and deprecation notice in some other place. If you want to make this a major update right away, we can also get rid of the fallback.

  • 🎉1
MoritzLost commented

@midzer Can you create a new release with the merged PR? The latest release v2.3.3 doesn't include it yet. Thanks!

midzer commented

https://github.com/midzer/tobii/releases/tag/v2.4.0 is out.

Let's leave this one open for an upcoming v3 when we can remove the backwards compatibility as a breaking change.

  • ❤️2
MoritzLost commented

@jakobhaerter This should probably be kept open as a reminder to drop the prefixes in 3.0.0 ;)

  • 👍1
jakobhaerter commented

@MoritzLost @midzer Yeah, my bad! 🙏