internal/flutter_availability
5
0
Fork 0
mirror of https://github.com/Iconica-Development/flutter_availability.git synced 2025-05-19 13:13:44 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

chore: apply code review comments

Browse source
This commit is contained in:
Joey Boerwinkel 2024-07-15 11:30:16 +02:00 committed by Freek van de Ven
parent bc32462c16
commit 3238fc2c4a
12 changed files with 67 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
packages/flutter_availability/lib/src/config/availability_options.dart
View file

@ -24,6 +24,7 @@ class AvailabilityOptions {
this.colors = const AvailabilityColors(),
this.confirmationDialogBuilder = DefaultConfirmationDialog.builder,
this.timePickerBuilder,
// TODO(Joey): Also have a DefaultLoader.builder
this.loadingIndicatorBuilder = defaultLoader,
AvailabilityDataInterface? dataInterface,
}) : dataInterface = dataInterface ?? LocalAvailabilityDataInterface();

2
packages/flutter_availability/lib/src/service/initial_data.dart
View file

@ -15,6 +15,8 @@ List<AvailabilityModel> getDefaultLocalAvailabilitiesForUser() {
AvailabilityModel(
id: availability.$1,
userId: "",
// TODO(Joey): Even though this is testdata, Never use .add(duration)
// to move to the next day.
startDate: currentMonth.add(Duration(days: availability.$2)),
endDate: currentMonth.add(Duration(days: availability.$2)),
breaks: [],

3
packages/flutter_availability/lib/src/service/local_data_interface.dart
View file

@ -207,6 +207,7 @@ class LocalAvailabilityDataInterface implements AvailabilityDataInterface {
return availabilities[index];
}
}
// TODO(Joey): Throw proper exceptions here
throw Exception("Availability not found");
}
@ -225,6 +226,8 @@ class LocalAvailabilityDataInterface implements AvailabilityDataInterface {
return templates[index];
}
}
// TODO(Joey): Add proper exceptions in the data interface and throw them
// here
throw Exception("Template not found");
}

17
packages/flutter_availability/lib/src/ui/screens/availability_modification.dart
View file

@ -54,6 +54,8 @@ class _AvailabilitiesModificationScreenState
@override
void initState() {
super.initState();
// TODO(Joey): These can be immediately assigned to the properties
// This removes the need for an initState
_availability =
widget.initialAvailabilities.getAvailabilities().firstOrNull ??
AvailabilityModel(
@ -76,6 +78,8 @@ class _AvailabilitiesModificationScreenState
// TODO(freek): the selected period might be longer than 1 month
//so we need to get all the availabilites through a stream
// TODO(Joey): separate logic from layout to adhere to the single
// responsibility principle
Future<void> onSave() async {
if (_clearAvailability) {
await service.clearAvailabilities(
@ -100,11 +104,15 @@ class _AvailabilitiesModificationScreenState
}
Future<void> onClickSave() async {
// TODO(Joey): The name confirmationDialogBuilder does not represent the
// expected implementation.
var confirmed = await options.confirmationDialogBuilder(
context,
title: translations.availabilityDialogConfirmTitle,
description: translations.availabilityDialogConfirmDescription,
);
// TODO(Joey): We should make the interface of the dialog function return
// a non nullable bool. Now we are implicitly setting default behaviour
if (confirmed ?? false) {
await onSave();
}
@ -121,6 +129,7 @@ class _AvailabilitiesModificationScreenState
var clearSection = AvailabilityClearSection(
range: widget.dateRange,
clearAvailable: _clearAvailability,
// TODO(Joey): Extract this function
onChanged: (isChecked) {
setState(() {
_clearAvailability = isChecked;
@ -130,6 +139,7 @@ class _AvailabilitiesModificationScreenState
var templateSelection = AvailabilityTemplateSelection(
selectedTemplates: _selectedTemplates,
// TODO(Joey): Extract this function
onTemplateAdd: () async {
var template = await widget.onTemplateSelection();
if (template != null) {
@ -138,6 +148,7 @@ class _AvailabilitiesModificationScreenState
});
}
},
// TODO(Joey): Extract these functions
onTemplatesRemoved: () {
setState(() {
_selectedTemplates = [];
@ -150,9 +161,11 @@ class _AvailabilitiesModificationScreenState
startTime: _startTime,
endTime: _endTime,
key: ValueKey([_startTime, _endTime]),
// TODO(Joey): Extract these
onStartChanged: (start) => setState(() {
_startTime = start;
}),
// TODO(Joey): Extract these
onEndChanged: (end) => setState(() {
_endTime = end;
}),
@ -160,6 +173,7 @@ class _AvailabilitiesModificationScreenState
var pauseSelection = PauseSelection(
breaks: _availability.breaks,
// TODO(Joey): Extract these
onBreaksChanged: (breaks) {
setState(() {
_availability = _availability.copyWith(breaks: breaks);
@ -167,6 +181,8 @@ class _AvailabilitiesModificationScreenState
},
);
// TODO(Joey): this structure is defined multiple times, we should create
// a widget to handle this consistently
var body = CustomScrollView(
slivers: [
SliverPadding(
@ -181,6 +197,7 @@ class _AvailabilitiesModificationScreenState
templateSelection,
const SizedBox(height: 24),
timeSelection,
// TODO(Joey): Not divisible by 4
const SizedBox(height: 26),
pauseSelection,
],

4
packages/flutter_availability/lib/src/ui/screens/availability_overview.dart
View file

@ -51,6 +51,7 @@ class _AvailabilityOverviewState extends State<AvailabilityOverview> {
var availabilitySnapshot = useStream(availabilityStream);
// TODO(Joey): Way too complex of a function
var selectedAvailabilities = _selectedRange != null
? availabilitySnapshot.data
?.where(
@ -93,6 +94,7 @@ class _AvailabilityOverviewState extends State<AvailabilityOverview> {
availabilities: availabilitySnapshot,
);
// TODO(Joey): too complex of a definition for the function
var onButtonPress = _selectedRange == null
? null
: () {
@ -111,6 +113,7 @@ class _AvailabilityOverviewState extends State<AvailabilityOverview> {
title: translations.clearAvailabilityConfirmTitle,
description: translations.clearAvailabilityConfirmDescription,
);
// TODO(Joey): Expect a non nullable
if (confirmed ?? false) {
await service
.clearAvailabilities(selectedAvailabilities.getAvailabilities());
@ -132,6 +135,7 @@ class _AvailabilityOverviewState extends State<AvailabilityOverview> {
Text(translations.editAvailabilityButton),
);
// TODO(Joey): This structure is defined multiple times
var body = CustomScrollView(
slivers: [
SliverPadding(

6
packages/flutter_availability/lib/src/ui/screens/template_day_modification.dart
View file

@ -104,12 +104,15 @@ class _DayTemplateModificationScreenState
var timeSection = TemplateTimeSelection(
key: ValueKey(_template.templateData),
// TODO(Joey): Extract this
startTime: TimeOfDay.fromDateTime(
(_template.templateData as DayTemplateData).startTime,
),
// TODO(Joey): Extract this
endTime: TimeOfDay.fromDateTime(
(_template.templateData as DayTemplateData).endTime,
),
// TODO(Joey): Extract this
onStartChanged: (start) {
var startTime = (_template.templateData as DayTemplateData).startTime;
var updatedStartTime = DateTime(
@ -127,6 +130,7 @@ class _DayTemplateModificationScreenState
);
});
},
// TODO(Joey): Extract this
onEndChanged: (end) {
var endTime = (_template.templateData as DayTemplateData).endTime;
var updatedEndTime = DateTime(
@ -148,6 +152,7 @@ class _DayTemplateModificationScreenState
var colorSection = TemplateColorSelection(
selectedColor: _selectedColor,
// TODO(Joey): Extract this
onColorSelected: (color) {
setState(() {
_selectedColor = color;
@ -158,6 +163,7 @@ class _DayTemplateModificationScreenState
var pauseSection = PauseSelection(
breaks: (_template.templateData as DayTemplateData).breaks,
// TODO(Joey): Extrac this
onBreaksChanged: (breaks) {
setState(() {
_template = _template.copyWith(

8
packages/flutter_availability/lib/src/ui/screens/template_overview.dart
View file

@ -100,6 +100,7 @@ class _TemplateListSection extends StatelessWidget {
final String sectionTitle;
final String createButtonText;
// transform the stream to a snapshot as low as possible to reduce rebuilds
final AsyncSnapshot<List<AvailabilityTemplateModel>> templatesSnapshot;
final void Function(AvailabilityTemplateModel template) onEditTemplate;
final VoidCallback onAddTemplate;
@ -147,8 +148,13 @@ class _TemplateListSection extends StatelessWidget {
Text(sectionTitle, style: textTheme.titleMedium),
const SizedBox(height: 8),
const Divider(height: 1),
// TODO(Joey): Do not make this nullable, in the build make sure to
// have the expected value ready.
for (var template
in templatesSnapshot.data ?? <AvailabilityTemplateModel>[]) ...[
// TODO(Joey): Extract this as a widget
// TODO(Joey): Do not simply use gesture detectors, always think of
// semantics, interaction and other UX
GestureDetector(
onTap: () => onClickTemplate(template),
child: Container(
@ -171,6 +177,8 @@ class _TemplateListSection extends StatelessWidget {
const SizedBox(width: 8),
Text(template.name, style: textTheme.bodyLarge),
const Spacer(),
// TODO(Joey): Do not simply use gesture detectors, always
// think of semantics, interaction and other UX
GestureDetector(
onTap: () => onEditTemplate(template),
child: const Icon(Icons.edit),

6
packages/flutter_availability/lib/src/ui/widgets/availability_template_selection.dart
View file

@ -35,6 +35,7 @@ class AvailabilityTemplateSelection extends StatelessWidget {
var options = availabilityScope.options;
var translations = options.translations;
// TODO(Joey): Do not nest ternairy operators
var titleText = selectedTemplates.isEmpty
? translations.availabilityAddTemplateTitle
: selectedTemplates.length > 1
@ -62,6 +63,7 @@ class AvailabilityTemplateSelection extends StatelessWidget {
if (selectedTemplates.isEmpty) ...[
addButton,
] else ...[
// TODO(Joey): Extract this as a widget
Container(
padding: const EdgeInsets.all(12),
decoration: BoxDecoration(
@ -69,6 +71,9 @@ class AvailabilityTemplateSelection extends StatelessWidget {
color: theme.colorScheme.primary,
width: 1,
),
// TODO(Joey): This seems like a repeating borderRadius. I can
// understand if these are not configurable, but I do think that
// they should be defined only once.
borderRadius: BorderRadius.circular(5),
),
child: Row(
@ -79,6 +84,7 @@ class AvailabilityTemplateSelection extends StatelessWidget {
crossAxisAlignment: CrossAxisAlignment.start,
children: [
for (var template in selectedTemplates) ...[
// TODO(Joey): Extract this as a widget
Row(
children: [
Container(

4
packages/flutter_availability/lib/src/ui/widgets/calendar_grid.dart
View file

@ -73,6 +73,7 @@ class CalendarGrid extends StatelessWidget {
mainAxisSpacing: 12,
),
itemBuilder: (context, index) {
// TODO(Joey): Extract all this as a widget
var day = calendarDays[index];
var dayColor = day.color ??
colors.customAvailabilityColor ??
@ -86,6 +87,7 @@ class CalendarGrid extends StatelessWidget {
);
var textStyle = textTheme.bodyLarge?.copyWith(color: textColor);
// TODO(Joey): Watch out for using gesture detectors
return GestureDetector(
onTap: () => onDayTap(day.date),
child: DecoratedBox(
@ -186,6 +188,8 @@ List<CalendarDay> _generateCalendarDays(
required bool isNextMonth,
}) {
for (var i = 0; i < count; i++) {
// TODO(Joey): Never increase days with duration, always use internal
// datetime math.
var day = isNextMonth
? startDay.add(Duration(days: i + 1))
: startDay.subtract(Duration(days: count - i));

2
packages/flutter_availability/lib/src/ui/widgets/pause_selection.dart
View file

@ -140,6 +140,7 @@ class BreakDisplay extends StatelessWidget {
TimeOfDay.fromDateTime(breakModel.endTime),
);
// TODO(Joey): Watch out with gesture detectors
return GestureDetector(
onTap: onClick,
child: Container(
@ -158,6 +159,7 @@ class BreakDisplay extends StatelessWidget {
"$endTime",
),
const Spacer(),
// TODO(Joey): Watch out with gesturedetectors
GestureDetector(onTap: onRemove, child: const Icon(Icons.remove)),
],
),

12
packages/flutter_availability/lib/src/ui/widgets/template_legend.dart
View file

@ -110,11 +110,15 @@ class _TemplateLegendState extends State<TemplateLegend> {
),
padding: const EdgeInsets.only(right: 2),
child: _templateDrawerOpen && !templatesLoading
// TODO(Joey): A listview inside a scrollview inside the
// scrollable that each page has seems like really strange UX
// TODO(Joey): No ternary operators in the layout
? SingleChildScrollView(
child: Column(
children: [
Container(
constraints: const BoxConstraints(
// TODO(Joey): Not divisible by 4
maxHeight: 150,
),
child: Scrollbar(
@ -125,9 +129,12 @@ class _TemplateLegendState extends State<TemplateLegend> {
child: ListView.builder(
controller: _scrollController,
shrinkWrap: true,
// TODO(Joey): This seems like an odd way to
// implement appending items
itemCount: templates.length + 2,
itemBuilder: (context, index) {
if (index == 0) {
// TODO(Joey): Extract this as a widget
return Column(
children: [
_TemplateLegendItem(
@ -175,6 +182,9 @@ class _TemplateLegendState extends State<TemplateLegend> {
thickness: 1,
),
),
// TODO(Joey): This is too complex of a check to read the layout
// There are 8 different combinations parameters with 2 different
// outcomes
if (!templatesAvailable &&
(!_templateDrawerOpen || templatesLoading)) ...[
const SizedBox(height: 12),
@ -213,6 +223,7 @@ class _TemplateLegendItem extends StatelessWidget {
Container(
decoration: BoxDecoration(
color: backgroundColor,
// TODO(Joey): Use a global borderRadius
borderRadius: BorderRadius.circular(5),
border: Border.all(
color: borderColor ?? Colors.transparent,
@ -221,6 +232,7 @@ class _TemplateLegendItem extends StatelessWidget {
width: 20,
height: 20,
),
// TODO(Joey): Not divisible by 4
const SizedBox(width: 6),
Text(name, style: Theme.of(context).textTheme.bodyLarge),
],

2
packages/flutter_availability/lib/src/util/availability_deviation.dart
View file

@ -11,6 +11,8 @@ bool isTemplateDeviated(
DateTime? templateStartDate;
DateTime? templateEndDate;
// TODO(Joey): Add a method to a templateModel: getEndTimeForDayOfWeek()
// as well as for start time. Allow polymorphism to resolve this if statement
if (template.templateType == AvailabilityTemplateType.week) {
templateStartDate = (template.templateData as WeekTemplateData)
.data[WeekDay.values[dayOfWeek - 1]]