ArdanaLabs / DanaSwapUI

Other
3 stars 3 forks source link

Theme as type + some collateral fixes #48

Closed toastal closed 2 years ago

toastal commented 2 years ago

In the FP space, booleans are bad as they are not explicit. There exists more problems with the booleans, namely that W3C does have future plans to extend the light/dark to more themes which we wouldn’t be handling. Also a boolean locks us into a binary theme (what if we want an OLED-friendly theme that is #000, etc.). The new getTheme function properly runs through the problem space with the fallback. Along with extending beyond two theme, the new structure can express unsetting an explicit value as there is no way to do this currently. isDarkTheme is still a useful variable around the app because the themes are tied to material UI styles though I think this is a bug too and they shouldn’t be arbitrarily limiting users either. Building our types around our tools instead of our ideas is not a great way to handle things. Their styles module seems deprecated in v5. Unsure, but maybe they saw this as flawed as well.

Personally I think the switch toggle is bad as it can’t unselect and is not future-proof—but this is the trend.

Collateral fixes::

toastal commented 2 years ago

@elite0226 if you get some time, can you actually run and test this for real in your browser? I have a sneaking suspicion that there may be something visually missed given the size of the change. I didn't see anything when I looked, but you know the UI better than I do.

elite0226 commented 2 years ago

Let me take a look at this. thanks.