added connected double lessons with own setting #49

Merged
MineTec merged 4 commits from develop-connectedDoubleLessons into develop 2024-03-31 10:50:59 +00:00
6 changed files with 96 additions and 5 deletions

@ -16,4 +16,15 @@ extension IsSameDay on DateTime {
TimeOfDay toTimeOfDay() {
return TimeOfDay(hour: hour, minute: minute);
}
bool isSameDateTime(DateTime other) {
bool isSameDay = this.isSameDay(other);
bool isSameTimeOfDay = (toTimeOfDay() == other.toTimeOfDay());
return isSameDay && isSameTimeOfDay;
}
bool isSameOrAfter(DateTime other) {
return isSameDateTime(other) || isAfter(other);
}
}

@ -4,7 +4,9 @@ part 'timetableSettings.g.dart';
@JsonSerializable()
class TimetableSettings {
TimetableSettings();
bool connectDoubleLessons;
TimetableSettings({required this.connectDoubleLessons});
factory TimetableSettings.fromJson(Map<String, dynamic> json) => _$TimetableSettingsFromJson(json);
Map<String, dynamic> toJson() => _$TimetableSettingsToJson(this);

@ -7,7 +7,11 @@ part of 'timetableSettings.dart';
// **************************************************************************
TimetableSettings _$TimetableSettingsFromJson(Map<String, dynamic> json) =>
TimetableSettings();
TimetableSettings(
connectDoubleLessons: json['connectDoubleLessons'] as bool,
);
Map<String, dynamic> _$TimetableSettingsToJson(TimetableSettings instance) =>
<String, dynamic>{};
<String, dynamic>{
'connectDoubleLessons': instance.connectDoubleLessons,
};

@ -244,9 +244,66 @@ class _TimetableState extends State<Timetable> {
];
}
List<GetTimetableResponseObject> _removeDuplicates(TimetableProps data, Duration maxTimeBetweenDouble) {
List<GetTimetableResponseObject> timetableList = data.getTimetableResponse.result.toList();
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
Review

klammern weg oder aufklappen mit leerzeichen zwischen bedingungsklammern und block

klammern weg **oder** aufklappen mit leerzeichen zwischen bedingungsklammern und block
if(timetableList.isEmpty) return timetableList;
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
timetableList.sort((a, b) => _parseWebuntisTimestamp(a.date, a.startTime).compareTo(_parseWebuntisTimestamp(b.date, b.startTime)));
GetTimetableResponseObject previousElement = timetableList.first;
for(var i = 1; i < timetableList.length; i++) {
GetTimetableResponseObject currentElement = timetableList.elementAt(i);
Pupsi marked this conversation as resolved
Review

das sollte umgeschrieben werden.
ggf musst du die pfeilsyntax in einen normalen curly braces block abändern und dort dann mit normalen ifs erst auf null prüfen - auch wenn das länger ist

das nullable mit dem fallback auf 1 == 2 ist sehr sehr hacky...

bool isSameSubject() {
  SomeNullableType? currentElementSubjects = currentElement.su.firstOrNull;
  SomeNullableType? previousElementSubjects = previousElement.su.firstOrNull;

  if(currentElementSubjects == null || previousElementSubjects == null) return false;
  return currentElementSubjects!.id == previousElementSubjects!.id;
}
das sollte umgeschrieben werden. ggf musst du die pfeilsyntax in einen normalen curly braces block abändern und dort dann mit normalen ifs erst auf null prüfen - auch wenn das länger ist das nullable mit dem fallback auf 1 == 2 ist sehr sehr hacky... ``` bool isSameSubject() { SomeNullableType? currentElementSubjects = currentElement.su.firstOrNull; SomeNullableType? previousElementSubjects = previousElement.su.firstOrNull; if(currentElementSubjects == null || previousElementSubjects == null) return false; return currentElementSubjects!.id == previousElementSubjects!.id; } ```
bool isSameLesson() {
Pupsi marked this conversation as resolved Outdated

findest du eine bessere Möglichkeit als das .add(const Duration(seconds: 1))?

findest du eine bessere Möglichkeit als das `.add(const Duration(seconds: 1))`?
int? currentSubjectId = currentElement.su.firstOrNull?.id;
int? previousSubjectId = previousElement.su.firstOrNull?.id;
Pupsi marked this conversation as resolved
Review

leerzeichen

leerzeichen
if(currentSubjectId == null || previousSubjectId == null || currentSubjectId != previousSubjectId) return false;
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved Outdated

durch das entfernen der zweiten schulstunde gehen potentiell wichtige Informationen verloren.

Was passiert wenn eine stunde der doppelstunde abgesagt ist?

  • Dann wäre entweder die doppelstunde abgesagt
  • oder die doppelstunde vollständig da, obwohl eine stunde davon ausfallen würde

mein Vorschlag hier ist entweder die prüfung von isSameSubject so anzupassen, das diese false liefert, wenn beide stunden nicht den selben status haben, oder du fügst eine dritte Bedingung hinzu wie z.B. isSameStatus()

zur Orientierung ob eine Stunde ausfällt oder nicht:

bool _isCrossedOut(CalendarAppointmentDetails calendarEntry) {

durch das entfernen der zweiten schulstunde gehen potentiell wichtige Informationen verloren. Was passiert wenn eine stunde der doppelstunde abgesagt ist? - Dann wäre entweder die doppelstunde abgesagt - oder die doppelstunde vollständig da, obwohl eine stunde davon ausfallen würde mein Vorschlag hier ist entweder die prüfung von `isSameSubject` so anzupassen, das diese false liefert, wenn beide stunden nicht den selben status haben, oder du fügst eine dritte Bedingung hinzu wie z.B. `isSameStatus()` zur Orientierung ob eine Stunde ausfällt oder nicht: https://mhsl.eu/gitea/MarianumMobile/Client/src/commit/75846750f7f1ea4e161825e9f3f3529e47cbdb57/lib/view/pages/timetable/timetable.dart#L327
int? currentRoomId = currentElement.ro.firstOrNull?.id;
int? previousRoomId = previousElement.ro.firstOrNull?.id;
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
if(currentRoomId != previousRoomId) return false;
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
int? currentTeacherId = currentElement.te.firstOrNull?.id;
int? previousTeacherId = previousElement.te.firstOrNull?.id;
if(currentTeacherId != previousTeacherId) return false;
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
String? currentStatusCode = currentElement.code;
String? previousStatusCode = previousElement.code;
if(currentStatusCode != previousStatusCode) return false;
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
return true;
}
bool isNotSeparated() => _parseWebuntisTimestamp(previousElement.date, previousElement.endTime).add(maxTimeBetweenDouble)
Pupsi marked this conversation as resolved Outdated

leerzeichen zu viel nach pfeilsyntax

leerzeichen zu viel nach pfeilsyntax

und weiter hinten ebenfalls

und weiter hinten ebenfalls
.isSameOrAfter(_parseWebuntisTimestamp(currentElement.date, currentElement.startTime));
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
if(isSameLesson() && isNotSeparated()) {
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
previousElement.endTime = currentElement.endTime;
timetableList.remove(currentElement);
Pupsi marked this conversation as resolved Outdated

leerzeichen

leerzeichen
i--;
} else {
previousElement = currentElement;
}
}
return timetableList;
}
TimetableEvents _buildTableEvents(TimetableProps data) {
List<Appointment> appointments = data.getTimetableResponse.result.map((element) {
List<GetTimetableResponseObject> timetableList = data.getTimetableResponse.result.toList();
Pupsi marked this conversation as resolved Outdated

leerzeichen zu viel

leerzeichen zu viel
if(settings.val().timetableSettings.connectDoubleLessons) {
timetableList = _removeDuplicates(data, const Duration(minutes: 5));
Pupsi marked this conversation as resolved Outdated

leerzeichen zu viel

leerzeichen zu viel
}
List<Appointment> appointments = timetableList.map((element) {
GetRoomsResponse rooms = data.getRoomsResponse;
GetSubjectsResponse subjects = data.getSubjectsResponse;

@ -23,7 +23,9 @@ class DefaultSettings {
askedForPreferredGradeSystem: false,
inputs: []
),
timetableSettings: TimetableSettings(),
timetableSettings: TimetableSettings(
connectDoubleLessons: false,
),
talkSettings: TalkSettings(
sortFavoritesToTop: true,
sortUnreadToTop: false,

@ -8,6 +8,7 @@ import 'package:provider/provider.dart';
import 'package:shared_preferences/shared_preferences.dart';
import '../../model/accountData.dart';
import '../../model/timetable/timetableProps.dart';
import '../../notification/notifyUpdater.dart';
import '../../storage/base/settingsProvider.dart';
import '../../theming/appTheme.dart';
@ -98,6 +99,20 @@ class _SettingsState extends State<Settings> {
const Divider(),
ListTile(
leading: const Icon(Icons.calendar_view_day_outlined),
Pupsi marked this conversation as resolved
Review

der Begriff ist glaube ich nicht ganz so super

was hälst du von Doppelstunden zusammenhängend anzeigen?

der Begriff ist glaube ich nicht ganz so super was hälst du von `Doppelstunden zusammenhängend anzeigen`?
title: const Text('Doppelstunden zusammenhängend anzeigen'),
trailing: Checkbox(
value: settings.val().timetableSettings.connectDoubleLessons,
onChanged: (e) {
Pupsi marked this conversation as resolved
Review

beim testen ist mir aufgefallen, dass beim Ändern der Einstellung diese nicht direkt sichtbar sind auf dem Stundenplan.

Dies führt zu Verwirrung, wenn man z.B. erst im Stundenplan scrollen/ neuladen muss um die verbunden Stunden sehen zu können.

der Grund dahinter ist, das die meisten Einstellungen durch ein einfaches widget rebuild sichtbar werden. Die logik für die vebrunden Stundenpläne wird aber nicht mit jedem widget-build aufgerufen, da dies extrem auf die performance gehen würde.
Daher musst du das "neubauen" des Stundenplans manuell triggern.
Du kannst an dieser Stelle nach dem setzen der Einstellung das so wie beim beim drag-to-reload des stundenplans machen.

Siehe:

Provider.of<TimetableProps>(context, listen: false).run(renew: true);

die Methode run() auf TimetableProps triggert den rebuilt

den parameter renew brauchst du nicht. (renew besagt das die daten nicht aus dem cache kommen dürfen und neu von webuntis heruntergeladen werden sollen)

beim testen ist mir aufgefallen, dass beim Ändern der Einstellung diese nicht direkt sichtbar sind auf dem Stundenplan. Dies führt zu Verwirrung, wenn man z.B. erst im Stundenplan scrollen/ neuladen muss um die verbunden Stunden sehen zu können. der Grund dahinter ist, das die meisten Einstellungen durch ein einfaches widget rebuild sichtbar werden. Die logik für die vebrunden Stundenpläne wird aber nicht mit jedem widget-build aufgerufen, da dies extrem auf die performance gehen würde. Daher musst du das "neubauen" des Stundenplans manuell triggern. Du kannst an dieser Stelle nach dem setzen der Einstellung das so wie beim beim drag-to-reload des stundenplans machen. Siehe: https://mhsl.eu/gitea/MarianumMobile/Client/src/commit/75846750f7f1ea4e161825e9f3f3529e47cbdb57/lib/view/pages/timetable/timetable.dart#L177 die Methode run() auf TimetableProps triggert den rebuilt den parameter renew brauchst du nicht. (renew besagt das die daten nicht aus dem cache kommen dürfen und neu von webuntis heruntergeladen werden sollen)
settings.val(write: true).timetableSettings.connectDoubleLessons = e!;
Provider.of<TimetableProps>(context, listen: false).run(renew: false);
},
),
),
const Divider(),
ListTile(
leading: const Icon(Icons.star_border),
title: const Text('Favoriten im Talk nach oben sortieren'),