Skip to content

Commit 1fb76ac

Browse files
gnpricePIG208
authored andcommitted
app_bar [nfc]: Explain why the lightbox skips ZulipAppBar
Just listing that there is an exception doesn't help much to provide guidance for someone writing a new page on whether they should also skip ZulipAppBar there. The "why" is much more helpful for that. Right at the call site is also more helpful than at the definition of ZulipAppBar, because the call site is the place someone will definitely be looking if they're contemplating whether this deviation from the pattern is appropriate, a mistake, or just an oversight.
1 parent 50d39ea commit 1fb76ac

File tree

2 files changed

+7
-2
lines changed

2 files changed

+7
-2
lines changed

lib/widgets/app_bar.dart

-2
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import 'store.dart';
55
/// A custom [AppBar] with a loading indicator.
66
///
77
/// This should be used for most of the pages with access to [PerAccountStore].
8-
// However, there are some exceptions (add more if necessary):
9-
// - `lib/widgets/lightbox.dart`
108
class ZulipAppBar extends AppBar {
119
ZulipAppBar({
1210
super.key,

lib/widgets/lightbox.dart

+7
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,13 @@ class _LightboxPageLayoutState extends State<_LightboxPageLayout> {
157157
.add_Hms()
158158
.format(DateTime.fromMillisecondsSinceEpoch(widget.message.timestamp * 1000));
159159

160+
// We use plain [AppBar] instead of [ZulipAppBar], even though this page
161+
// has a [PerAccountStore], because:
162+
// * There's already a progress indicator with a different meaning
163+
// (loading the image).
164+
// * The app bar can be hidden, so wouldn't always be visible anyway.
165+
// * This is a page where the store loading indicator isn't especially
166+
// necessary: https://github.com/zulip/zulip-flutter/pull/852#issuecomment-2264211917
160167
appBar = AppBar(
161168
centerTitle: false,
162169
foregroundColor: appBarForegroundColor,

0 commit comments

Comments
 (0)