Skip to content

Conversation

@yo1995
Copy link
Contributor

@yo1995 yo1995 commented Aug 13, 2025

Description

Close #1104. This PR fixes a problem with the refreshable modifier. It used to give a console warning:

Attempting to change the refresh control while it is not idle is strongly discouraged and probably won't work properly.

which indicates that the task in refreshable was canceled and destroyed prematurely (or the environment RefreshAction was changed in an unexpected way).

Previously, this was mitigated by using an unstructured Task block, as its lifespan isn't affected by the view; However, it still leads to the drag-down refresh animation to be cut short.

Proposed Fix

The root cause of the bug was a messed lifecycle of the refreshable task. isLoadingModels boolean was immediately set to true when the loadModels() method is called, causing the progress view to show up and destroy the refreshable modifier. Instead of using a conditional view, i.e.,

if condA {
    ProgressView()
} else {
    switch mode {
        // cases for various views
    }
}

moving everything into the switch-case allows the refreshable to stay put while the mode is being determined.

Similarly, I moved the offlineDisabledView into the switch-case. It doesn't have to be moved but I figured why not - now all views are tidily nested under the same level.

To Discuss

  • With the changes, the UI behavior is different

Previously, whenever we pull to refresh, the progress view will be there for the entire time of loadModels(). Now, it only appears on the first method call. Is it OK?

  • Instead of moving progress view into switch-case, an alternative is to move it to an overlay. Do you prefer this approach?

@yo1995 yo1995 marked this pull request as draft August 13, 2025 23:46
@yo1995 yo1995 changed the title offline: Condense conditions into mode offline: Consolidate conditions into mode Aug 14, 2025
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