-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add config to worldclkinfo #4037
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
base: master
Are you sure you want to change the base?
Add config to worldclkinfo #4037
Conversation
|
LGTM, @RKBoss6 would you fancy a bit of testing? |
|
Of course! @chuckwagoncomputing Is it set up on your app loader? That way I can test the customizer as well. Thanks! |
|
Yes it's currently up at https://chuckwagoncomputing.github.io/BangleApps/ It might not consistently stay up because I'm messing around with some other apps, but it is right now. |
|
I gave it some testing, and the uploads all look good, but when you tap on the ClockInfo where it used to show the city name, now it shows something like Edit: It looks like there are many different modes it can render the string as, what do we need all of those for? Should we only keep the name and time? |
|
Also, just bump the version in metadata up, and add an entry in changelog :) |
|
Oh yeah, I forgot about the version number. The different modes allow flexibilty for different personal prefernces and use cases. I like to see the day of the week. |
|
I found and fixed a bug: The default settings values were different between clkinfo.js and settings.js, which resulted in "shorten cities" being enabled by default, but showing as not enabled in settings. Toggling in settings got things back in sync, but this should be fixed now. |
For me, I would say most of the modes look the same, and having direct access to the city name is much more efficient for me. Perhaps we could make it a setting, for only city name and time, or the full 5 modes? |
|
Also, another bug I found was if you have a clock with more than one clock info and both are set to the same city (eg. London), then both are updated when you tap on one, rather than only that one being rendered with the next mode |
Hmmm, I think this could be done...
This I already knew, it's a consequence of the mode being saved for persistence. I didn't see it as a bug because I couldn't think of a use case for having two of the same city on the same screen. |
|
Simple mode now implemented. I don't know how to fix the other thing without removing persistence. |
|
Hmm, in the olde version, it also had that error, and I never came across it, so that should actually be fine. I'll get to testing this out later today :) |
|
Alright, tested and it all looks good! The only issue I see that's not in the older one is when you tapped to toggle between city name and time, if you swiped down, to change cities, that would stay persistent, showing the clock or city throughout changes. That goes away now, and some show city names, some show time... |
|
That is intended. While I can't imagine a use case for having two of the same city in different modes, I can imagine a use case for having different cities in different modes, so I made the persistence city-specific. |
|
Hmm, I feel like it would make selection process less efficient, as you need to swipe, tap a few times, swipe again, and repeat, just to get to the city you want. Could we bundle the persistence into having simple mode off? |
818acd1 to
c42c1bf
Compare
|
I'm not sure if I completely understand what you're asking for, but give it a try now and let me know what you think. |
|
That's much better! I'd say once you take a look at the reviews, this is good to go!! |
|
Sorry if you couldn't understand what I was trying to day earlier, I think you got the gist of it, but effectively I preferred having the persistence global, so tapping once sets all of the clockinfos to city names, and tapping again sets all back to time regardless of scrolling through. Tying it into the simple mode is a good change, as users can now choose whether they like the old interface, or a newer one! Nice job, thank you! |
Re: #4033
@RKBoss6
This should be feature-complete compared to the original.