Skip to content

Refactor `AppListItemController` to improve updates tab stability

There have been a host of stability problems around the updates tab.

A lot of it revolves around the complexity of AppListItemController. What seemed as a good idea at the time (code reuse for common layouts - i.e. those with an app icon, name,, and perhaps a button or progress indicator) has already grown into something unmaintainable. As such, this redesigns the class to make it more understandable, maintainable, and theoretically it is now quite testable if somebody felt the desire to write tests for the UI too.

The main problem being addressed, is that different parts of the UI are updated in response to different events. For example, a download event may cause just the progress bar to show and update, but it doesn't actively hide unrelated widgets. Instead, it just presumes that they were correctly hidden earlier. This results in oddities like the progress bar being shown behind the "Update" button after a download is complete.

You can think of it as:

  • Callack 1:
    • Update widget X:
      • Is app in state a, b, or c?
    • Update widget Y:
      • Is app in state b or c?
    • Update widget Z:
      • Is app in state a?
  • Callback 2:
  • Update widget x:
    • Is app in state a or b?
  • Update widget Z:
    • Is app in state c?

It is too easy to have each widget updated in a manner which is incongruous with the other widgets, because they don't interrogate the apps state consistently.

The new design works by only having one method responsible for updating the view, and each event/callback/whatever which wants to update the view is responsible for invoking that. It also separates the business logic from the UI code so that they can both be audited separately. This is done by having the business logic write to a really dump AppListItemState class which is only responsible for representing the current state of the UI.

You can think of this as:

  • Callback 1:
    • Refresh view:
      • Is app in state a, b, or c?
        • Update widget X
        • Update widget Y
        • Update widget Z
  • Callback 2:
    • Refresh view:
      • Is app in state a?
        • Update widget X
        • Update widget Y
        • Update widget Z

Here each single callback results in updating all the widgets. This is much more predictable. If it becomes a performance problem then we can look at optimizing it, but anecdotally it doesn't seem problematic.

After working with it during refactoring, I think this is much better. It also behaves much better than it used to when using the updates tab.

Edited by username-removed-25042

Merge request reports