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

gallery not updating list based on sort #618

Conversation

krugerk
Copy link
Contributor

@krugerk krugerk commented Jan 20, 2025

Summary

Gallery was not updating how its list was sorted after the preference had been adjusted. Also, the gallery's last updated was meant to display when the goals were last fetched.

The gallery was not monitoring for changes to the sorting preference. It needed a trigger to sort the list anew. This was removed in d181dba. 'Update goals' can stay, it is the fetch goals that was to be reduced. Furthermore, since gallery's last updated is meant to indicate how fresh the data is (when they were last fetched), gallery's setting this to now where it did does not necessarily coincide with fetching the goals anew. Instead, the goal manager already provides this nugget.

For UI changes including screenshots of before and after is great.

Validation

Ran app in simulator.
gallery -> settings -> sort pref, picked another, and back to gallery from settings
Noticed sort had taken effect.
Also noticed last updated seems much more up-to-date and less out of sync, as reported in #588.

@krugerk krugerk requested a review from a team as a code owner January 20, 2025 10:22
@krugerk krugerk requested review from theospears and removed request for a team January 20, 2025 10:22
Copy link
Collaborator

@theospears theospears left a comment

Choose a reason for hiding this comment

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

Thanks for noticing this and fixing it. Let's remove the unrelated change but otherwise looks good. I've also poked the tests as I think the failure was spurious.

gallery is not monitoring sort pref
and with a change introduced in d181dba
the sort was not being applied when returning to the gallery
after changing the preference.
The point was to not fetch goals anew automatically each time
the gallery came into view.
gallery was setting its own date at last updated though this
easily could be out of sync with when the goals were actually last
fetched.
instead the goal manager already provides this piece of data.
@krugerk krugerk force-pushed the bugfix/gallery-not-updating-list-based-on-sort branch from 6e7b166 to 930ae2b Compare January 26, 2025 09:02
@krugerk
Copy link
Contributor Author

krugerk commented Jan 26, 2025

Thanks for noticing this and fixing it. Let's remove the unrelated change but otherwise looks good. I've also poked the tests as I think the failure was spurious.

Non-essential changes have been removed. The tests may also have been failing due to UI updates on a mon UI thread. Fixed.

@krugerk krugerk requested a review from theospears January 26, 2025 09:05
@theospears theospears merged commit ef2855d into beeminder:master Jan 27, 2025
1 check passed
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.

2 participants