Skip to content

Conversation

@AuxiliumCDNG
Copy link
Contributor

Closes #56
Please run some thorough testing. I have no idea if it works reliably tbh. xD

Also added a reload button and changed call structure a bit for easier data updating
You can choose if you want to save the to date
The rest gets saved and restored automatically
TODO: Add delta-timespan when ready
@AuxiliumCDNG AuxiliumCDNG added the new feature The will become a new feature label May 6, 2022
fabiancdng and others added 4 commits May 6, 2022 20:56
(Hopefully) improved type-checking logic, naming and some comments
for the code.
Front End: Added delta chart and delta settings
- Unset localStorage items if user opted-out
of localStorage using checkbox
- Add global variable 'saveInLocalStorage' representing whether
or not settings are supposed to be saved in localStorage
- Fix issues with seperate scale switch for weight in compare chart
@fabiancdng
Copy link
Member

I ran into some issues with the separate scale switch + I added some logic to unset the items in localStorage (other than daterange-save-to) when the user disabled the feature. Hope my changes are fine with you. Other than that, very well done, great feature!

@AuxiliumCDNG
Copy link
Contributor Author

AuxiliumCDNG commented May 7, 2022

You use firefox, right?
Why you read an unset item, does it return null as in not set or "null" as a string?
Chromium seems to do the latter. Is that the/a problem? Cause it worked for me obviously...
That's why the if clauses are a bit weird in my version...

@fabiancdng
Copy link
Member

@AuxiliumCDNG Oh, that's interesting... Both Chrome and Firefox (on macOS) return null (the type) if the item is not set, not a string. But considering it seems to be a string for you, we might want to switch to localStorage.hasOwnProperty('key') for checking whether the item is set in the storage. This should (in every browser) result in either true or false being returned.... I love JavaScript :)

@AuxiliumCDNG
Copy link
Contributor Author

AuxiliumCDNG commented May 7, 2022

What about other datatypes? Are booleans still booleans when you read them? They get "converted" to strings in my browser (Vivaldi). Thats why bools are supposed to be 0/1 (which is also a string 😂).
I will run some tests this evening and commit a fix (hopefully). Oh dear...

@AuxiliumCDNG
Copy link
Contributor Author

AuxiliumCDNG commented May 7, 2022

Hope my changes are fine with you.

Your changes break the intended use of daterange-save-to which is to keep the "end-date" up to date but keep all other settings. "to" not "too".
That's handy when you load the site days later to see current data. It shouldn't delete all settings.

Have a look at https://github.com/Programmier-AG/BeeLogger/wiki/Miscellaneous#localstorage-keys

@AuxiliumCDNG
Copy link
Contributor Author

I would actually like to roll back and fix what's wrong in place. Or if you want to fix your version, you can do that. Your decision.

@fabiancdng
Copy link
Member

@AuxiliumCDNG Reverted my changes, the branch is now back to where you left off.

@AuxiliumCDNG
Copy link
Contributor Author

I will try to come up with a fix. The same browser on windows 11 instead of 10 behaves as your browsers do btw. I will try to make it as redundant and "javascript-save" as possible.

@AuxiliumCDNG AuxiliumCDNG changed the base branch from master to frontend-improvements May 7, 2022 19:14
@fabiancdng
Copy link
Member

fabiancdng commented May 7, 2022

@AuxiliumCDNG I don't know if this helps but JSON.parse("null"); and JSON.parse(null); should both result in null (the type; not string). Maybe you can use that to fix the issue.

@AuxiliumCDNG
Copy link
Contributor Author

Please hang on while I test the merge...

@AuxiliumCDNG
Copy link
Contributor Author

AuxiliumCDNG commented May 7, 2022

@fabiancdng Please verify everything is working. Then feel free to merge.
Should be safe to delete...

@fabiancdng
Copy link
Member

@AuxiliumCDNG It works for me. Only the checkbox to opt-in or out of saving the settings in the localStorage does nothing (at least in my browser) 😕 If I uncheck it, the values are still being stored and read from the localStorage.

Screenshot 2022-05-08 at 00 32 05

@AuxiliumCDNG
Copy link
Contributor Author

Uncheck it, change end date to yesterday, apply, reload the page and check if the end date changes. Does it change?

@AuxiliumCDNG
Copy link
Contributor Author

Works for me now in Vivaldi 5.2.2623.41 (Stable channel) (64-Bit), which is the current build.
Also tested in Chrome Version 101.0.4951.54 (Offizieller Build) (64-Bit)
and Firefox 100.0 (64-Bit).

Copy link
Member

@fabiancdng fabiancdng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AuxiliumCDNG Okay for me to merge then, thanks! It worked for me too except for the checkbox weirdness... But maybe that was just Firefox caching input states across tabs or me being dumb, who knows

@AuxiliumCDNG
Copy link
Contributor Author

Uncheck it, change end date to yesterday, apply, reload the page and check if the end date changes. Does it change?

@fabiancdng Did you do that?

@fabiancdng
Copy link
Member

@AuxiliumCDNG Tried that both with the start and end date. It didn't change back when the checkbox was unchecked. Only tested it with Firefox though

@AuxiliumCDNG
Copy link
Contributor Author

@fabiancdng Could you run localStorage.clear() and try again?

@fabiancdng
Copy link
Member

@AuxiliumCDNG Oh haha works now 😅 Is it intentional that only the end date resets and not the start date?

@AuxiliumCDNG
Copy link
Contributor Author

Hope my changes are fine with you.

Your changes break the intended use of daterange-save-to which is to keep the "end-date" up to date but keep all other settings. "to" not "too". That's handy when you load the site days later to see current data. It shouldn't delete all settings.

Have a look at https://github.com/Programmier-AG/BeeLogger/wiki/Miscellaneous#localstorage-keys

@fabiancdng It is

@fabiancdng
Copy link
Member

@AuxiliumCDNG Sorry, overlooked that. I understand.

@fabiancdng fabiancdng merged commit cb9a4f1 into frontend-improvements May 8, 2022
@fabiancdng fabiancdng deleted the local-storage branch May 8, 2022 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature The will become a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use localStorage to save the state of the 'seperate weight' switch and date range

3 participants