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

refactor: tray and window behaviours #1298

Merged
merged 7 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/dart/npt_flutter/lib/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class App extends StatelessWidget {

/// A cubit which manages the system tray entries
BlocProvider<TrayCubit>(
create: (_) => TrayCubit()..initialize(),
create: (_) => TrayCubit(),
Copy link
Member Author

Choose a reason for hiding this comment

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

This gets called in a more appropriate place now.

),

/// A bloc which manages favorites
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import 'dart:async';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:npt_flutter/app.dart';
import 'package:npt_flutter/features/favorite/favorite.dart';
import 'package:npt_flutter/features/tray_manager/tray_manager.dart';

part 'favorite_event.dart';
part 'favorite_state.dart';
Expand Down Expand Up @@ -32,7 +31,6 @@ class FavoriteBloc extends LoggingBloc<FavoriteEvent, FavoritesState> {
return;
}
emit(FavoritesLoaded(favs.values));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
Copy link
Member Author

@XavierChanth XavierChanth Aug 30, 2024

Choose a reason for hiding this comment

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

Decoupling - remove all "pushed" updates to the tray.

}

FutureOr<void> _onAdd(
Expand All @@ -44,7 +42,6 @@ class FavoriteBloc extends LoggingBloc<FavoriteEvent, FavoritesState> {
emit(FavoritesLoaded(
[...(state as FavoritesLoaded).favorites, event.favorite],
));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
try {
await _repo.addFavorite(event.favorite);
} catch (_) {}
Expand All @@ -63,7 +60,6 @@ class FavoriteBloc extends LoggingBloc<FavoriteEvent, FavoritesState> {
.difference(event.toRemove.toSet()),
));

App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
try {
var profileIds = <String>{};
for (Favorite fav in event.toRemove) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ sealed class Favorite extends Loggable {
Future<String?> get displayName;
String? get status;
Iterable<String> get profileIds;

/// These 4 functions are for interacting with favorites in the tray,
/// since the implementation may differ by type of favoritable
bool isFavoriteMatch(Favoritable favoritable);
bool isLoadedInProfiles(Iterable<String> profiles);
bool containsProfile(String uuid);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'package:npt_flutter/app.dart';
import 'package:npt_flutter/features/profile/profile.dart';
import 'package:npt_flutter/features/profile_list/profile_list.dart';
import 'package:npt_flutter/features/settings/settings.dart';
import 'package:npt_flutter/features/tray_manager/tray_manager.dart';
import 'package:socket_connector/socket_connector.dart';

part 'profile_event.dart';
Expand Down Expand Up @@ -38,12 +37,10 @@ class ProfileBloc extends LoggingBloc<ProfileEvent, ProfileState> {

if (profile == null) {
emit(ProfileFailedLoad(uuid));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
Copy link
Member Author

Choose a reason for hiding this comment

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

More of the same

return;
}

emit(ProfileLoaded(uuid, profile: profile));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
}

Future<void> _onLoadOrCreate(
Expand All @@ -70,12 +67,10 @@ class ProfileBloc extends LoggingBloc<ProfileEvent, ProfileState> {
localPort: 0,
),
));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
return;
}

emit(ProfileLoaded(uuid, profile: profile));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
}

Future<void> _onEdit(
Expand Down Expand Up @@ -119,7 +114,6 @@ class ProfileBloc extends LoggingBloc<ProfileEvent, ProfileState> {
.invalidate(uuid);
emit(ProfileFailedSave(uuid, profile: event.profile));
}
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
}

Future<void> _onStart(
Expand Down Expand Up @@ -214,7 +208,6 @@ class ProfileBloc extends LoggingBloc<ProfileEvent, ProfileState> {
// Save the socket connector to state so it can be used to stop npt later
App.navState.currentContext?.read<ProfilesRunningCubit>().cache(uuid, sc);
emit(ProfileStarted(uuid, profile: profile));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
} catch (err) {
cancelSubs?.call();
emit(ProfileFailedStart(
Expand All @@ -229,7 +222,6 @@ class ProfileBloc extends LoggingBloc<ProfileEvent, ProfileState> {
?.read<ProfilesRunningCubit>()
.invalidate(uuid);
emit(ProfileLoaded(uuid, profile: profile));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:npt_flutter/app.dart';
import 'package:npt_flutter/features/favorite/favorite.dart';
import 'package:npt_flutter/features/profile/profile.dart';
import 'package:npt_flutter/features/tray_manager/tray_manager.dart';

part 'profile_list_event.dart';
part 'profile_list_state.dart';
Expand All @@ -31,18 +30,15 @@ class ProfileListBloc extends LoggingBloc<ProfileListEvent, ProfileListState> {

if (profiles == null) {
emit(const ProfileListFailedLoad());
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
return;
}

emit(ProfileListLoaded(profiles: profiles));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
}

Future<void> _onUpdate(
ProfileListUpdateEvent event, Emitter<ProfileListState> emit) async {
emit(ProfileListLoaded(profiles: event.profiles));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
}

Future<void> _onDelete(
Expand All @@ -69,7 +65,6 @@ class ProfileListBloc extends LoggingBloc<ProfileListEvent, ProfileListState> {
unawaited(_repo.deleteProfile(uuid));
}
bloc?.add(FavoriteRemoveEvent(favoritesToRemove));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
}

Future<void> _onAdd(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:npt_flutter/app.dart';
import 'package:npt_flutter/features/tray_manager/tray_manager.dart';
import 'package:socket_connector/socket_connector.dart';

part 'profiles_running_state.dart';
Expand All @@ -10,11 +8,9 @@ class ProfilesRunningCubit extends LoggingCubit<ProfilesRunningState> {

void cache(String uuid, SocketConnector connector) {
emit(state.withConnector(uuid, connector));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
}

void invalidate(String uuid) {
emit(state.withoutConnector(uuid));
App.navState.currentContext?.read<TrayCubit>().reloadFavorites();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,34 +87,50 @@ class TrayCubit extends LoggingCubit<TrayState> {
emit(const TrayLoaded());
}

Future<void> reloadFavorites() async {
Future<void> reload() async {
Copy link
Member Author

Choose a reason for hiding this comment

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

No functional changes to the reload tray method, but I did document it for clarity

var context = App.navState.currentContext;
if (context == null) return;
var init = initialize();

/// Access the context before any awaited function calls
var showSettings = context.read<OnboardingCubit>().state is Onboarded;
var favoriteBloc = context.read<FavoriteBloc>();
var profilesList = context.read<ProfileListBloc>();
if (state is TrayInitial) {
await initialize();
}

await init;

/// Get favorites
if (favoriteBloc.state is! FavoritesLoaded) return;
var favorites = (favoriteBloc.state as FavoritesLoaded).favorites;

/// Get profiles uuid list
if (profilesList.state is! ProfileListLoaded) return;
var profiles = (profilesList.state as ProfileListLoaded).profiles;

/// Generate the new menu based on current state
var favMenuItems = await Future.wait(
favorites.where((e) => e.isLoadedInProfiles(profiles)).map((e) async {
favorites
.where((fav) => fav.isLoadedInProfiles(profiles))
.map((fav) async {
/// Make sure to call [e.displayName] and [e.isRunning] only once to
/// ensure good performance - these getters call a bunch of nested
/// information from elsewhere in the app state
var displayName = await e.displayName;
var status = e.status;
var displayName = await fav.displayName;
var status = fav.status;
var label = '$displayName $status';
return MenuItem(
label: label,
toolTip: status,
onClick: (_) => e.toggle(),
onClick: (_) => fav.toggle(),
);
}),
);

/// PERF: We should conditionally call setContextMenu if there was a state
/// change which resulted in an actual change to the favorites list.
/// Currently we just force call updates which is really inefficient
Comment on lines +129 to +131
Copy link
Member Author

@XavierChanth XavierChanth Aug 30, 2024

Choose a reason for hiding this comment

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

The lists that we iterate through are not expected to be long, but in the future, if someone were to have 1000s (probably an even bigger magnitude) of profiles loaded in the app we will have to determine if the tray reloading needs to be optimized - reducing useless reloads may help


/// Set the new menu
await trayManager.setContextMenu(Menu(
items: [
...favMenuItems,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import 'package:flutter/widgets.dart';
import 'package:flutter_bloc/flutter_bloc.dart';
import 'package:npt_flutter/features/favorite/favorite.dart';
import 'package:npt_flutter/features/profile/profile.dart';
import 'package:npt_flutter/features/profile_list/profile_list.dart';
import 'package:npt_flutter/features/tray_manager/tray_manager.dart';
import 'package:tray_manager/tray_manager.dart';
import 'package:window_manager/window_manager.dart';
Expand All @@ -18,11 +21,60 @@ class _TrayManagerState extends State<TrayManager>
with TrayListener, WindowListener {
@override
Widget build(BuildContext context) {
var cubit = context.read<TrayCubit>();
if (cubit.state is TrayInitial) {
cubit.initialize();
var trayCubit = context.read<TrayCubit>();
if (trayCubit.state is TrayInitial) {
trayCubit.initialize();
}
return widget.child;

/// Must strongly type [context] here or Dart will infer the wrong type for
/// the [.read()] extension which causes an error
void reloadTray(BuildContext context, _) {
context.read<TrayCubit>().reload();
}

var profileCacheCubit = context.read<ProfileCacheCubit>();

/// This selector reduces the number of times we reload profiles-map
return BlocSelector<ProfileListBloc, ProfileListState, Iterable<String>>(
selector: (state) {
if (state is ProfileListLoaded) return state.profiles;
return List.empty();
},
builder: (context, profiles) => MultiBlocListener(
listeners: [
/// Reload the tray whenever one of the following states changes
/// Note: this doesn't always result in a change to the tray, but we
/// still have to check
BlocListener<FavoriteBloc, FavoritesState>(
listener: reloadTray,
),
BlocListener<ProfileListBloc, ProfileListState>(
listener: reloadTray,
),
BlocListener<ProfilesRunningCubit, ProfilesRunningState>(
listener: reloadTray,
),

/// Yeah I really hate this... an indefinite list of listeners
/// but it's the only way to decouple the profiles from having to know
/// about the tray
///
/// The tray should know about profiles, profiles should not know
/// about tray. Even if it is slightly more costly in performance, the
/// calls where we take the performance hit are:
/// 1. In an asynchronous background task (who cares)
/// 2. Worth it, compared to the potential maintenance costs
...profiles.map((uuid) => BlocProvider<ProfileBloc>(
key: Key("TrayManager-$uuid"),
create: (context) => profileCacheCubit.getProfileBloc(uuid),
child: BlocListener<ProfileBloc, ProfileState>(
listener: reloadTray,
),
)),
],
Comment on lines +44 to +74
Copy link
Member Author

Choose a reason for hiding this comment

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

All of these are the new listeners which will trigger the tray to reload whenever any of these change - could be an edited profile, favorited / unfavorited, start / stop, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

I dislike that we need to iterate through each profile and add it as a listener, we could optimize by filtering to only favorited profiles, but that introduces edge cases not worth dealing with right now.

child: widget.child,
),
);
}

@override
Expand Down
10 changes: 9 additions & 1 deletion packages/dart/npt_flutter/lib/main.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:developer';

import 'package:flutter/material.dart';
import 'package:window_manager/window_manager.dart';

Expand All @@ -6,5 +8,11 @@ import 'app.dart';
Future<void> main() async {
WidgetsFlutterBinding.ensureInitialized();
windowManager.ensureInitialized();
runApp(const App());
try {
await windowManager.setSkipTaskbar(true); // Don't show the app icon in dock
Copy link
Member Author

Choose a reason for hiding this comment

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

Disable dock icon - the dock icon wasn't functional, and made it look like the app is hung, but really the app is only functional from the tray.

} catch (_) {
log("Failed to setSkipTaskbar");
} finally {
runApp(const App());
}
}