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

[mtnclock] Fix widgets disappearing on weather update #3586

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gloriousDan
Copy link

Fixes #3441

I found the issue for why the widget bar keeps disappearing in the mountain clock app.
It's related to the weather update but the bug is within mtnclock's app.js file.

The widget setting is written within the mtnclock.json file. The weather data is written to the same file. When a weather update comes in, the file is overwritten and the showWidgets setting consequently deleted from the file which leads to the widget bar disappearing.

This should fix it. We could also read the current state of data before writing it but in my testing it worked fine like this.

@gloriousDan
Copy link
Author

You can try it out on my app loader: https://gloriousdan.github.io/BangleApps/?q=mtnclock

@bobrippling
Copy link
Collaborator

Sounds good, thanks for the fix - looks like there's two lint errors if you're ok to sort those out? What do you think to merging the whole file each time, like a sort of:

writeJson(
  "mtnclock.json",
  Object.assign(
    readJson("mtnclock.json"),
    newSettings,
  )
);

@gloriousDan gloriousDan force-pushed the fix-mtnclock-widgets branch 2 times, most recently from 3d519f8 to bda48c6 Compare September 25, 2024 17:01
@gloriousDan
Copy link
Author

gloriousDan commented Sep 25, 2024

Good point, I thought about that too but at first implemented the simpler fix. But now I implemented your recommendations.

The linter issues should also be fixed now. Unfortunately I didn't get to set up the tests locally, so let's see if the workflows succeed this time :)

Edit: I now checked it in the espruino IDE and there were no errors anymore

@gloriousDan
Copy link
Author

Can I somehow get the CI to run within in my fork?

@gloriousDan gloriousDan marked this pull request as draft September 26, 2024 13:09
@bobrippling bobrippling marked this pull request as ready for review September 27, 2024 07:55
@bobrippling
Copy link
Collaborator

Sure, I've kicked off CI here if that helps, it was because the PR was in draft. Those changes look great btw, I'll merge if you're happy with everything?

@gloriousDan
Copy link
Author

Thanks, I marked it as draft because I thought of a potential error I still wanted to check/ fix. Maybe you could also help me determine wether it's a problem or not.

Since I load set the updated weather data not to the data object anymore, but to the newSettings and then write it to the mtnclock.json, I suspect that the clock only shows the updated data when the app is reloaded again. In my code the data object is only ever read from mtnclock.json at the start of the app and not updated later.

Is my understanding correct, that line 1 is only run once at the start?

I probably get around to fixing this soon, and afterwards we can merge it.

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

Successfully merging this pull request may close these issues.

[mtnclock] Widgets disappear on Mountain Pass Clock
2 participants