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

Displays moon as 100%@0:08 for two consecutive nights #572

Closed
xandro0777 opened this issue Mar 16, 2022 · 8 comments · Fixed by #686
Closed

Displays moon as 100%@0:08 for two consecutive nights #572

xandro0777 opened this issue Mar 16, 2022 · 8 comments · Fixed by #686
Labels
Milestone

Comments

@xandro0777
Copy link

right now, March 17th and 18th in central European winter time. Don't think that can be correct.. looks like time zone confusion?

Assuming the time means transit time there should not be any on the 17th.

@forrestguice
Copy link
Owner

forrestguice commented Mar 17, 2022

You are correct there isn't any transit on the 17th - the times you see are both for the 18th.

The app needs to pick a single illumination value for a given day (and typically that's lunar noon) but in this case its "lunar noon tomorrow". This wasn't really an issue before the app also displayed @time afterward, but now it seems a little misleading.

I'm not sure what the best solution is.. maybe just omit the @time part when the transit doesn't occur, because the illumination value itself (as a daily value) is accurate.

EDIT.. another view of this edge case:

Screenshot_2022-03-16-18-20-41

@xandro0777
Copy link
Author

xandro0777 commented Mar 17, 2022 via email

@forrestguice
Copy link
Owner

forrestguice commented Mar 17, 2022

Yes, it is clearly wrong at the indicated time - the problem is we're showing a time without indicating its tomorrow. That @time was added because people were unclear about why the illumination on the main page was different from the dialog, but now it reveals this edge case (and how the app currently deals with it).

The illumination on the 17th and 18th is 100% when the moon is at its highest - on the 17th that just happens to occur on the wrong side of the date line (from timezone setting) - those dates basically share a lunar noon.

The suggested solutions are good. Displaying a range is one possibility (that I'm liking more and more). Also displaying the illumination at solar noon would make the problem 'go away', but as noted it would probably confuse people. I'm still leaning towards just omitting the @time - at least one day out of every month would just show % without any @time afterward - the remainder would indicate time of lunar noon.

@forrestguice
Copy link
Owner

hrm, this pattern always coincides with a full moon? Its the difference between showing 99% vs 100.

@xandro0777
Copy link
Author

xandro0777 commented Mar 17, 2022 via email

@xandro0777
Copy link
Author

xandro0777 commented Mar 17, 2022 via email

@forrestguice
Copy link
Owner

That's the conclusion I've come to as well - it would be better to always display a range (and just eliminate any special cases). That range of values is wider (more interesting/useful) depending on the day.

The transit time doesn't appear anywhere else though, so continuing to display it means new UI somewhere.
I think these changes are something for v0.15.0 (along with other enhancements to the moon dialog). The next patch will just be the band-aid solution mentioned before (hiding the @time), a quick change to stop the app from miscommunicating the transit time.

@xandro0777
Copy link
Author

xandro0777 commented Mar 18, 2022 via email

forrestguice added a commit that referenced this issue Mar 29, 2022
omit lunar noon from the main card on days it doesn't occur (instead of displaying time of tomorrow's event) (#572)
forrestguice added a commit that referenced this issue Apr 6, 2022
omit lunar noon from the main card on days it doesn't occur (instead of displaying time of tomorrow's event) (#572)
@forrestguice forrestguice added this to the v0.14.3 milestone Apr 6, 2022
forrestguice added a commit that referenced this issue Nov 11, 2022
main card displays moon illumination range (#572)
@forrestguice forrestguice mentioned this issue Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants