model: Track/memoize stream-color variants somewhere other than `/api/model`

[chrisbobbe]

Currently (well, after #381), the Subscription class has a colorSwatch method to return a memoized StreamColorSwatch. That code is in /api/model.

Probably there's a better, more UI-focused place to do those computations (adjusting lightness, opacity, etc.) and provide them to consumers: https://github.com/zulip/zulip-flutter/pull/381#discussion_r1396528348

[chrisbobbe]

What if we made one simple global cache, as a Map<int, StreamColorSwatch> keyed by Subscription.color values? This might make sense, because a StreamColorSwatch isn't stateful and it's computed purely from a Subscription.color value.

When UI code is processing a subscription with .color 0xff76ce90, it can call a function that adds an entry for 0xff76ce90 if it's missing, or reads it if it's already present, and returns the corresponding StreamColorSwatch.

It's possible for that entry to fall into disuse at some point during the app session; for example, if

• the stream's channel's color is changed
• the user leaves the channel
• the user logs out of the org with the channel

But I don't see really significant downsides to that.

[gnprice]

Those leaks should be basically fine in practice, since those changes aren't common and since this cache wouldn't outlast the current process. Still, it'd be nicer to have something that doesn't leak.

How about just indexing on the stream ID? (As part of the PerAccountStore.) Then there'd be no leaks.

If the user has lots of subscriptions sharing the same color, then that'd give up opportunities to reuse the results. But that's probably not common, and in any event the worst case is just one StreamColorSwatch per subscription.

[chrisbobbe]

It would be nice for UI elements to consume these color swatches via AnimatedTheme / ThemeExtension, so they would animate between their light- and dark-mode colors when you toggle dark mode, just like other style attributes that are consumed via AnimatedTheme. If the cache is global instead of per-account, the natural way to do that is through MaterialApp.theme, which configures an AnimatedTheme built by MaterialApp.

In principle we should be able to instantiate per-account AnimatedThemes in our own code. Maybe that could be a job of PerAccountStoreWidget? That widget is meant to be lightweight, though; would it be OK if it created an AnimatedTheme for its subtree? Its .data would have to come from Theme.of(context).copyWith(extensions: [/* … */]), I think. I recently used ThemeData.copyWith for an "inner ThemeData" in a non-final revision of #635 (see https://github.com/zulip/zulip-flutter/pull/635#discussion_r1581581049). I remember it felt unsatisfying not to define all our theme data in one place, though.

…Hmm. I guess one concrete downside of putting an AnimatedTheme in every PerAccountStoreWidget is that, when dark mode is toggled, the animation will be accomplished by calling ThemeData.lerp redundantly for each page in the nav, when it could instead have been called only for the one AnimatedTheme near the root. That could get expensive, especially when the stream-color-swatch cache is large.

[chrisbobbe]

If the cache were indexed by Subscription.color ints, I guess someone could artificially inflate the cache for people by using the Zulip API to rapidly change a single channel's color through some millions of colors. I don't know how realistically this could become a denial of service (probably they'd hit server rate limits first) but it would be easy to mitigate/discourage by limiting the cache size.

[gnprice]

Yeah, I'd prefer to avoid having PerAccountStoreWidget itself cause any nontrivial computations.

The global cache indexed by Subscription.color seems fine, if that's the easiest solution.