-
Notifications
You must be signed in to change notification settings - Fork 5
Feature: Dim eReader screen #215
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
base: main
Are you sure you want to change the base?
Changes from all commits
e9675d4
d4221e7
c8bd105
be64f67
f87e725
2461e2f
1dcb0aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import 'package:kover/utils/constants/kover_icons.dart'; | |
| import 'package:kover/utils/layout_constants.dart'; | ||
| import 'package:kover/widgets/settings/boolean_option.dart'; | ||
| import 'package:kover/widgets/settings/choice_option.dart'; | ||
| import 'package:kover/widgets/settings/dim_option.dart'; | ||
| import 'package:kover/widgets/settings/numeric_option.dart'; | ||
| import 'package:kover/widgets/util/async_value.dart'; | ||
| import 'package:lucide_icons_flutter/lucide_icons.dart'; | ||
|
|
@@ -150,6 +151,7 @@ class ImageReaderSettingsBottomSheet extends ConsumerWidget { | |
| .read(provider.notifier) | ||
| .setVerticalReaderPadding(newValue), | ||
| ), | ||
| const DimOption(), | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Places here it would only be shown when the image reader is in vertical mode ;) Would personally place it above the |
||
| NumericOption( | ||
| title: 'Vertical Gap', | ||
| icon: LucideIcons.unfoldVertical, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import 'package:kover/riverpod/providers/reader//reader.dart'; | |
| import 'package:kover/riverpod/providers/reader/epub_reader.dart'; | ||
| import 'package:kover/riverpod/providers/reader/reader_navigation.dart'; | ||
| import 'package:kover/riverpod/providers/router.dart'; | ||
| import 'package:kover/riverpod/providers/settings/reader_dim_settings.dart'; | ||
| import 'package:kover/utils/layout_constants.dart'; | ||
| import 'package:kover/utils/logging.dart'; | ||
| import 'package:kover/widgets/util/async_value.dart'; | ||
|
|
@@ -95,6 +96,13 @@ class ReaderOverlay extends HookConsumerWidget { | |
| ), | ||
| ); | ||
|
|
||
| final dimLevel = ref | ||
| .watch(readerDimSettingsProvider) | ||
| .maybeWhen( | ||
| data: (state) => state.dimLevel, | ||
| orElse: () => 0.0, | ||
| ); | ||
|
|
||
| ref.listen( | ||
| readerNavigationProvider( | ||
| seriesId: seriesId, | ||
|
|
@@ -141,21 +149,36 @@ class ReaderOverlay extends HookConsumerWidget { | |
| }, | ||
| child: Stack( | ||
| children: [ | ||
| Positioned.fill( | ||
| child: Column( | ||
| mainAxisSize: .min, | ||
| children: [ | ||
| Expanded(child: child), | ||
| if (showProgressBar && state.series.format == .epub) | ||
| Positioned.fill(child: child), | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think a more elegant approach could be to extract the responsibility to a thin consumer widget which would watch the setting and apply the opacity to the child widget. That way the child could be wrapped within the |
||
| if (dimLevel > 0) | ||
| Positioned.fill( | ||
| child: IgnorePointer( | ||
| child: ColoredBox( | ||
| color: Color.fromRGBO(0, 0, 0, dimLevel), | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it work to just manipulate the child opacity through either an If not it could still be moved to a nested stack in the dedicated widget I think to avoid additional complexity in this already quite cluttered tree |
||
| ), | ||
| ), | ||
| ), | ||
| if (showProgressBar && state.series.format == .epub) | ||
| Positioned( | ||
| bottom: 0, | ||
| left: 0, | ||
| right: 0, | ||
| child: | ||
| SubpageProgress( | ||
| seriesId: seriesId, | ||
| chapterId: chapterId, | ||
| ) | ||
| .animate( | ||
| target: uiVisible.value ? 0.0 : 1.0, | ||
| ) | ||
| .fadeIn(duration: 200.ms) | ||
| else if (showProgressBar) | ||
| .fadeIn(duration: 200.ms), | ||
| ) | ||
| else if (showProgressBar) | ||
| Positioned( | ||
| bottom: 0, | ||
| left: 0, | ||
| right: 0, | ||
| child: | ||
| ReaderProgress( | ||
| seriesId: seriesId, | ||
| chapterId: chapterId, | ||
|
|
@@ -164,9 +187,7 @@ class ReaderOverlay extends HookConsumerWidget { | |
| target: uiVisible.value ? 0.0 : 1.0, | ||
| ) | ||
| .fadeIn(duration: 200.ms), | ||
| ], | ||
| ), | ||
| ), | ||
| Positioned.fill( | ||
| child: Row( | ||
| children: [ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import 'package:kover/utils/constants/kover_icons.dart'; | |
| import 'package:kover/utils/layout_constants.dart'; | ||
| import 'package:kover/widgets/settings/boolean_option.dart'; | ||
| import 'package:kover/widgets/settings/choice_option.dart'; | ||
| import 'package:kover/widgets/settings/dim_option.dart'; | ||
| import 'package:kover/widgets/util/async_value.dart'; | ||
|
|
||
| class PdfReaderSettingsBottomSheet extends ConsumerWidget { | ||
|
|
@@ -110,6 +111,7 @@ class PdfReaderSettingsBottomSheet extends ConsumerWidget { | |
| .setShowProgressBar(newValue); | ||
| }, | ||
| ), | ||
| const DimOption(), | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also this one to be moved to whatever the final position in the list will be |
||
| ], | ||
| ), | ||
| ), | ||
|
|
||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the idea in general for it to be a more generic setting, though for all other reader ones they're stored per series with the possibility of overriding the defaults. Since this will ultimately live in the same settings lists I think that behavior should be kept or it may become confusing. So far I just duplicated the setting in all respective models to avoid dealing with rippling the default overriding and resetting. I think it could either accept a series ID and have the defaults logic rippling and potentially find some elegant pattern that could be applied to the other common parameter, or be "duplicated" to the respective model while keeping the limits constants to somewhere central. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import 'package:freezed_annotation/freezed_annotation.dart'; | ||
| import 'package:hooks_riverpod/experimental/persist.dart'; | ||
| import 'package:kover/riverpod/repository/storage_repository.dart'; | ||
| import 'package:kover/utils/logging.dart'; | ||
| import 'package:riverpod_annotation/experimental/json_persist.dart'; | ||
| import 'package:riverpod_annotation/riverpod_annotation.dart'; | ||
|
|
||
| part 'reader_dim_settings.freezed.dart'; | ||
| part 'reader_dim_settings.g.dart'; | ||
|
|
||
| sealed class ReaderDimSettingsLimits { | ||
| static const double dimMin = 0.0; | ||
| static const double dimMax = 0.9; | ||
| static const double dimStep = 5.0; | ||
| } | ||
|
|
||
| @freezed | ||
| sealed class ReaderDimSettingsState with _$ReaderDimSettingsState { | ||
| const ReaderDimSettingsState._(); | ||
| const factory ReaderDimSettingsState({ | ||
| @Default(0.0) double dimLevel, | ||
| }) = _ReaderDimSettingsState; | ||
|
|
||
| factory ReaderDimSettingsState.fromJson(Map<String, Object?> json) => | ||
| _$ReaderDimSettingsStateFromJson(json); | ||
| } | ||
|
|
||
| @riverpod | ||
| @JsonPersist() | ||
| class ReaderDimSettings extends _$ReaderDimSettings { | ||
| @override | ||
| Future<ReaderDimSettingsState> build() async { | ||
| await persist( | ||
| ref.watch(storageProvider.future), | ||
| options: const StorageOptions(cacheTime: StorageCacheTime.unsafe_forever), | ||
| ).future; | ||
| return state.value ?? const ReaderDimSettingsState(); | ||
| } | ||
|
|
||
| Future<void> adjustDimLevel(double delta) async { | ||
| final current = await future; | ||
| state = AsyncData( | ||
| current.copyWith( | ||
| dimLevel: (current.dimLevel + delta).clamp( | ||
| ReaderDimSettingsLimits.dimMin, | ||
| ReaderDimSettingsLimits.dimMax, | ||
| ), | ||
| ), | ||
| ); | ||
| log.info( | ||
| 'adjust dim level', | ||
| attributes: {'value': .double(state.value!.dimLevel)}, | ||
| ); | ||
| } | ||
|
|
||
| Future<void> setDimLevel(double level) async { | ||
| final current = await future; | ||
| state = AsyncData( | ||
| current.copyWith( | ||
| dimLevel: level.clamp( | ||
| ReaderDimSettingsLimits.dimMin, | ||
| ReaderDimSettingsLimits.dimMax, | ||
| ), | ||
| ), | ||
| ); | ||
| log.info( | ||
| 'set dim level', | ||
| attributes: {'value': .double(level)}, | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| import 'package:flutter/material.dart'; | ||
| import 'package:hooks_riverpod/hooks_riverpod.dart'; | ||
| import 'package:kover/riverpod/providers/settings/reader_dim_settings.dart'; | ||
| import 'package:kover/widgets/settings/numeric_option.dart'; | ||
| import 'package:lucide_icons_flutter/lucide_icons.dart'; | ||
|
|
||
| class DimOption extends ConsumerWidget { | ||
| const DimOption({super.key}); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context, WidgetRef ref) { | ||
| final dimLevel = ref | ||
| .watch(readerDimSettingsProvider) | ||
| .maybeWhen( | ||
| data: (state) => state.dimLevel, | ||
| orElse: () => 0.0, | ||
| ); | ||
|
|
||
| return NumericOption( | ||
| title: 'Screen Dimming', | ||
| icon: LucideIcons.sunMedium, | ||
| value: dimLevel * 100, | ||
| min: 0, | ||
| max: 90, | ||
| step: ReaderDimSettingsLimits.dimStep, | ||
| decimalPlaces: 0, | ||
| onChanged: (newValue) => ref | ||
| .read(readerDimSettingsProvider.notifier) | ||
| .setDimLevel(newValue / 100), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be good to have it in a position generally similar across the readers, probably right above the
show progress barwould be best