darkModeOn cookie return boolean, not string (enhancement) about halfmoon HOT 7 CLOSED

Fair enough! You know, this struck me as something that is so obvious to do initially, I hadn't considered that there may be a good reason you couldn't.

I'll close. Thanks for educating me!

from halfmoon.

@edadams yes cookies and localStorage can only store Strings, but you could parse/serialize the bool in the isDarkModeOn/setDarkModeOn methods. Which is a lot cleaner than for the user to check for yes and know the name of the cookie (darkMode).

Example (cookie, could also be implemented with localStorage 😉):

function isDarkModeOn() {
  return this.readCookie('darkModeOn') == 'yes';
}
function setDarkModeOn(darkMode) {
  this.createCookie('darkModeOn', darkMode ? 'yes' : 'no', 365);
}

I would use theme, which is generally used in application/other UI frameworks.

from halfmoon.

@edadams @TheKingDave
The dark mode cookie implementation has been overhauled in v1.1.0. There is a new method that returns the preferred mode. Again, this returns a string and not a boolean because there are 3 possible values, not just true or false.

Moreover, the setDarkModeOn method is not needed, because there is already halfmoon.toggleDarkMode(). Please refer to this page: https://www.gethalfmoon.com/docs/core-javascript-library/

The docs have been fleshed out, and hopefully everything should be clear now. Regarding the localStorage, I explained my reasoning why that will not be implemented here: #30 (comment)

I will go ahead and close this issue. You can re-open it if you think the issue is still not resolved, or open another issue.

from halfmoon.

Hey thanks for bringing this up. Cookie values can only be string. Libraries often allow cookies to store JSON and other data types (such as boolean), but that requires extra parsing before storage and after retrieval. For that reason, "yes" and "no" strings are used. This way, no parsing is required, and the cookie can be read directly.

from halfmoon.

I understand the reasoning that the readCookie method returns a string but I would at least create a isDarkModeOn method that returns a boolean. Perhaps also a setDarkModeOn method. These could be used internally. I would not store the darkMode in a String but in a bool. Only in the exception that you would implement themes in the future but then I would rename it to theme and store light and dark.

from halfmoon.

@TheKingDave Correct me if I've got this wrong but isn't the whole thing that you literally can't store a boolean value in a cookie? My understanding of the answer I got is that a cookie must return a string.

👍 +1 for theme that returns either light or dark. Could be uiTheme or some other more specific name, e.g,m mode or uiMode. (Is the accepted language "dark theme" or "dark mode"?)

That would be a separate issue, I think.

Edit: Guess you could even make an argument for using localStorage instead of cookies at this point, eh? 😉

from halfmoon.

Riiight! I'm with you now. I've reopened this issue in the hopes it gets looked at since I agree now that you've explained it this way. Hopefully that'll get someone's attention.

from halfmoon.