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.

Sign up for GitHub

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

Add VisualDensitySetting class. #624

Merged
merged 2 commits into from
Apr 23, 2023
Merged

Add VisualDensitySetting class. #624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7778e5a
Add `VisualDensitySetting` class.
Jonas-Sander Apr 19, 2023
a94fa0f
Fix test code.
Jonas-Sander Apr 19, 2023
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
125 changes: 108 additions & 17 deletions app/lib/account/theme/theme_settings.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class ThemeSettings extends ChangeNotifier {
required double defaultTextScalingFactor,

/// The value assigned to [visualDensity] if no other value is cached.
required VisualDensity defaultVisualDensity,
required VisualDensitySetting defaultVisualDensity,

/// The value assigned to [themeBrightness] if no other value is cached.
required ThemeBrightness defaultThemeBrightness,
Expand All @@ -96,7 +96,7 @@ class ThemeSettings extends ChangeNotifier {
keyValueStore.tryGetDouble(currentTextScalingFactorCacheKey) ??
defaultTextScalingFactor;

_visualDensity = keyValueStore
_visualDensitySetting = keyValueStore
.tryGetString(currentVisualDensityCacheKey)
.toVisualDensity() ??
defaultVisualDensity;
Expand All @@ -121,20 +121,20 @@ class ThemeSettings extends ChangeNotifier {
));
}

late VisualDensity _visualDensity;
VisualDensity get visualDensity => _visualDensity;
set visualDensity(VisualDensity value) {
_visualDensity = value;
late VisualDensitySetting _visualDensitySetting;
VisualDensitySetting get visualDensitySetting => _visualDensitySetting;
set visualDensitySetting(VisualDensitySetting value) {
_visualDensitySetting = value;
notifyListeners();

_keyValueStore.setString(currentVisualDensityCacheKey, value.serialize());
_analytics.log(NamedAnalyticsEvent(
name: 'ui_visual_density_changed',
data: {
'visual_density': {
'horizontal': value.horizontal,
'vertical': value.vertical
}
// bool is not allowed in firebase_analytics so we use `toString`.
'isAdaptivePlatformDensity': value.isAdaptivePlatformDensity.toString(),
'horizontal': value.visualDensity.horizontal,
'vertical': value.visualDensity.vertical
},
Comment on lines 133 to 138
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The old code would have always thrown an error here since the map inside map ('data:': { 'visual_density': {...}}) is not allowed by firebase_analytics.

));
}
Expand All @@ -153,27 +153,118 @@ class ThemeSettings extends ChangeNotifier {
}
}

extension on VisualDensity {
extension on VisualDensitySetting {
String serialize() {
return jsonEncode({
'horizontal': horizontal,
'vertical': vertical,
'isAdaptivePlatformDensity': isAdaptivePlatformDensity,
'horizontal': visualDensity.horizontal,
'vertical': visualDensity.vertical,
});
}
}

extension on String? {
VisualDensity? toVisualDensity() {
VisualDensitySetting? toVisualDensity() {
if (this == null) return null;
final _map = jsonDecode(this!) as Map;

return VisualDensity(
horizontal: _map['horizontal'] as double,
vertical: _map['vertical'] as double,
if (_map['isAdaptivePlatformDensity'] != null &&
_map['isAdaptivePlatformDensity'] == true) {
// We return [VisualDensitySetting.adaptivePlatformDensity] (which calls
// [VisualDensity.adaptivePlatformDensity]) instead of using the cached
// values because theoretically Flutter might change the default values of
// [VisualDensity.adaptivePlatformDensity] in the future.
return VisualDensitySetting.adaptivePlatformDensity();
}

return VisualDensitySetting.manual(
VisualDensity(
horizontal: _map['horizontal'] as double,
vertical: _map['vertical'] as double,
),
);
}
}

/// The current [VisualDensity].
///
/// This extra class is necessary to differentiate if a user has chosen
/// [VisualDensity.adaptivePlatformDensity] or a manual setting which happens to
/// be the same value as [VisualDensity.adaptivePlatformDensity] (one can't
/// tell by just looking at [VisualDensity]).
///
/// This allows the user choose between different [VisualDensity] values and the
/// adaptive platform density.
///
/// For example: On desktop the default density is [VisualDensity.compact], i.e.
/// [VisualDensity.adaptivePlatformDensity] will return [VisualDensity.compact].
/// Without this class we couldn't easily know if the user just uses the value
/// returned by [VisualDensity.adaptivePlatformDensity] or if
/// [VisualDensity.compact] was chosen manually.
/// This would mean that we couldn't easily highlight the current setting in the
/// UI (since it could either be "default"
/// [VisualDensity.adaptivePlatformDensity] or "compact"
/// [VisualDensity.compact].)
class VisualDensitySetting {
final VisualDensity visualDensity;
final bool isAdaptivePlatformDensity;

/// Corresponds to [VisualDensity.adaptivePlatformDensity].
///
/// This will automatically use the platform's default density by setting
/// [visualDensity] to [VisualDensity.adaptivePlatformDensity].
/// [isAdaptivePlatformDensity] will be true.
VisualDensitySetting.adaptivePlatformDensity()
: visualDensity = VisualDensity.adaptivePlatformDensity,
isAdaptivePlatformDensity = true;

/// Corresponds to [VisualDensity.standard].
///
/// [isAdaptivePlatformDensity] will be false.
VisualDensitySetting.standard()
: visualDensity = VisualDensity.standard,
isAdaptivePlatformDensity = false;

/// Corresponds to [VisualDensity.compact].
///
/// [isAdaptivePlatformDensity] will be false.
VisualDensitySetting.compact()
: visualDensity = VisualDensity.compact,
isAdaptivePlatformDensity = false;

/// Corresponds to [VisualDensity.comfortable].
///
/// [isAdaptivePlatformDensity] will be false.
VisualDensitySetting.comfortable()
: visualDensity = VisualDensity.comfortable,
isAdaptivePlatformDensity = false;

/// Use a custom [VisualDensity].
///
/// [isAdaptivePlatformDensity] will be false.
VisualDensitySetting.manual(this.visualDensity)
: isAdaptivePlatformDensity = false;

@override
bool operator ==(Object other) {
if (identical(this, other)) return true;

return other is VisualDensitySetting &&
other.visualDensity == visualDensity &&
other.isAdaptivePlatformDensity == isAdaptivePlatformDensity;
}

@override
int get hashCode {
return Object.hash(visualDensity, isAdaptivePlatformDensity);
}

@override
String toString() {
return 'VisualDensitySetting(visualDensity: $visualDensity, isAdaptivePlatformDensity: $isAdaptivePlatformDensity)';
}
}

extension on ThemeBrightness {
String serialize() {
return {
Expand Down
7 changes: 4 additions & 3 deletions app/lib/main/sharezone.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class _ThemeSettingsProvider extends StatelessWidget {
analytics: blocDependencies.analytics,
defaultTextScalingFactor: 1.0,
defaultThemeBrightness: ThemeBrightness.system,
defaultVisualDensity: VisualDensity.adaptivePlatformDensity,
defaultVisualDensity: VisualDensitySetting.adaptivePlatformDensity(),
keyValueStore: blocDependencies.keyValueStore,
),
child: Consumer<ThemeSettings>(builder: (context, themeSettings, _) {
Expand All @@ -173,8 +173,9 @@ class _ThemeSettingsProvider extends StatelessWidget {
textScaleFactor: themeSettings.textScalingFactor,
),
child: Theme(
data: Theme.of(context)
.copyWith(visualDensity: themeSettings.visualDensity),
data: Theme.of(context).copyWith(
visualDensity:
themeSettings.visualDensitySetting.visualDensity),
child: child,
),
);
Expand Down
8 changes: 4 additions & 4 deletions app/lib/main/sharezone_material_app.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ class SharezoneMaterialApp extends StatelessWidget {
@override
Widget build(BuildContext context) {
final themeSettings = context.watch<ThemeSettings>();
final _darkTheme =
darkTheme.copyWith(visualDensity: themeSettings.visualDensity);
final _lightTheme =
lightTheme.copyWith(visualDensity: themeSettings.visualDensity);
final _darkTheme = darkTheme.copyWith(
visualDensity: themeSettings.visualDensitySetting.visualDensity);
final _lightTheme = lightTheme.copyWith(
visualDensity: themeSettings.visualDensitySetting.visualDensity);

return FeatureDiscovery(
child: MaterialApp(
Expand Down
29 changes: 19 additions & 10 deletions app/test/theme/theme_settings_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,15 @@ void main() {
analytics: analytics,
defaultTextScalingFactor: 1.2,
defaultThemeBrightness: ThemeBrightness.dark,
defaultVisualDensity: VisualDensity.comfortable,
defaultVisualDensity:
VisualDensitySetting.manual(VisualDensity.comfortable),
keyValueStore: keyValueStore,
);

expect(themeSettings.textScalingFactor, 1.2);
expect(themeSettings.themeBrightness, ThemeBrightness.dark);
expect(themeSettings.visualDensity, VisualDensity.comfortable);
expect(themeSettings.visualDensitySetting,
VisualDensitySetting.manual(VisualDensity.comfortable));
});

test(
Expand All @@ -48,34 +50,39 @@ void main() {
analytics: analytics,
defaultTextScalingFactor: 1.2,
defaultThemeBrightness: ThemeBrightness.dark,
defaultVisualDensity: VisualDensity.comfortable,
defaultVisualDensity:
VisualDensitySetting.manual(VisualDensity.comfortable),
keyValueStore: keyValueStore,
);

// Will write to cache internally
themeSettings.textScalingFactor = 1.5;
themeSettings.themeBrightness = ThemeBrightness.light;
themeSettings.visualDensity = VisualDensity.compact;
themeSettings.visualDensitySetting =
VisualDensitySetting.manual(VisualDensity.compact);

themeSettings = ThemeSettings(
analytics: analytics,
defaultTextScalingFactor: 1.2,
defaultThemeBrightness: ThemeBrightness.dark,
defaultVisualDensity: VisualDensity.comfortable,
defaultVisualDensity:
VisualDensitySetting.manual(VisualDensity.comfortable),
keyValueStore: keyValueStore,
);

expect(themeSettings.textScalingFactor, 1.5);
expect(themeSettings.themeBrightness, ThemeBrightness.light);
expect(themeSettings.visualDensity, VisualDensity.compact);
expect(themeSettings.visualDensitySetting,
VisualDensitySetting.manual(VisualDensity.compact));
});

test('Tracks value changes via analytics', () {
final themeSettings = ThemeSettings(
analytics: analytics,
defaultTextScalingFactor: 1.2,
defaultThemeBrightness: ThemeBrightness.dark,
defaultVisualDensity: VisualDensity.comfortable,
defaultVisualDensity:
VisualDensitySetting.manual(VisualDensity.comfortable),
keyValueStore: keyValueStore,
);

Expand All @@ -89,11 +96,13 @@ void main() {
'ui_brightness_changed': {'brightness': 'system'}
});

themeSettings.visualDensity =
VisualDensity(horizontal: 1.0, vertical: 1.5);
themeSettings.visualDensitySetting = VisualDensitySetting.manual(
VisualDensity(horizontal: 1.0, vertical: 1.5));
expect(localAnalyticsBackend.loggedEvents[2], {
'ui_visual_density_changed': {
'visual_density': {'horizontal': 1.0, 'vertical': 1.5}
'isAdaptivePlatformDensity': 'false',
'horizontal': 1.0,
'vertical': 1.5
}
});
});
Expand Down