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

http://umbrel.local/settings don't save the config #1893

Open
TZocker opened this issue Aug 11, 2024 · 7 comments
Open

http://umbrel.local/settings don't save the config #1893

TZocker opened this issue Aug 11, 2024 · 7 comments

Comments

@TZocker
Copy link

TZocker commented Aug 11, 2024

I have the problem, that the temperature and language isn't saved permanently.

Everytime it is at default value.

It is stored, only for the browser session .

settings

@JoseMoranUrena523
Copy link

So I’m pretty sure your issue is related to this file. I might do a pull request later this weekend or next week and see if it can be included in something like umbrel.yaml.

@JoseMoranUrena523
Copy link

JoseMoranUrena523 commented Aug 17, 2024

The CPU setting looks like its being saved correctly, but I see the issue with the language setting noted in #1899. Make sure your browser allows local storage and session storage to be saved. If the pull request gets merged into the master branch and you keep getting issues, let me know and I'll continue looking or hopefully a developer at Umbrel can see this.

@TZocker
Copy link
Author

TZocker commented Aug 17, 2024

The CPU setting looks like its being saved correctlye, but I see the issue with the language setting noted in #1899. Make sure your browser allows local storage and session storage to be saved. If the pull request gets merged into the master branch and you keep getting issues, let me know and I'll continue looking or hopefully a developer at Umbrel can see this.

Yes this is correct but, i think the temperature setting is missing (F° vs. C°).

THX

@JoseMoranUrena523
Copy link

I'll take a look at the code for temperature in maybe an hour.

@JoseMoranUrena523
Copy link

So the specific code that handles that is this:

export function useTempUnit(optionalUnit?: TempUnit): [unit: TempUnit, setTemp: (unit: TempUnit) => void] {
	// Get default unit from the user's language
	const [langCode] = useLanguage()
	const langUnit = langCodesWithFahrenheitTemp.includes(langCode) ? 'f' : 'c'

	const defaultUnit = optionalUnit ?? langUnit

	// Not setting a default in `useLocalStorage2` because it would set the local storage value, which we don't want.
	// We want the unit to depend on the user language until the user specifically toggles to the a different temp unit.
	// Once they set the unit they want, we don't wanna change it even if they change the language.
	const [unit, setUnit] = useLocalStorage2<TempUnit>('temp-unit')

	return [unit ?? defaultUnit, setUnit]
}

The way that it's handled is that it gets the temperature setting (C or F) based on your language or based on a setting saved in window.LocalStorage (represented by the useLocalStorage2 function). Me personally, I don't see any issue with the code, but it might be best for an Umbrel developer to check it out.

@TZocker
Copy link
Author

TZocker commented Aug 17, 2024

Thx for the fast code review.

But we have to recheck the behavior if #1899 is fixed.

Until that i have everytime a language change ...

@JoseMoranUrena523
Copy link

And we can’t set up a development environment sadly. Let’s hope that fixes one part of the problem (although Umbrel staff is taking a while).

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