MarianumMobile/Client
MarianumMobile/Client
5
1
Fork 0
Code Issues 33 Pull Requests Actions Packages Projects 1 Releases Wiki Activity

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
Conversation 24 Commits 4 Files Changed 6 +96 -5
Pupsi commented 2024-03-30 17:29:25 +00:00
Member
Copy Link
No description provided.
Pupsi added 1 commit 2024-03-30 17:29:25 +00:00
MineTec reviewed 2024-03-30 18:03:56 +00:00
lib/view/pages/timetable/timetable.dart
@ -246,0 +247,4 @@
List<GetTimetableResponseObject> timetableList = data.getTimetableResponse.result.toList();
if(timetableList.isEmpty){ return timetableList; }
MineTec commented 2024-03-30 18:03:56 +00:00
Owner
Copy Link

klammern weg oder aufklappen mit leerzeichen zwischen bedingungsklammern und block

klammern weg **oder** aufklappen mit leerzeichen zwischen bedingungsklammern und block
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 18:28:02 +00:00
lib/view/pages/timetable/timetable.dart
@ -246,0 +255,4 @@
for (var i = 1; i < timetableList.length; i++) {
GetTimetableResponseObject currentElement = timetableList.elementAt(i);
bool isSameSubject() => (currentElement.su.firstOrNull?.id ?? 1) == (previousElement.su.firstOrNull?.id ?? 2);
MineTec commented 2024-03-30 18:28:02 +00:00
Owner
Copy Link

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; } ```
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 18:30:51 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -246,0 +256,4 @@
GetTimetableResponseObject currentElement = timetableList.elementAt(i);
bool isSameSubject() => (currentElement.su.firstOrNull?.id ?? 1) == (previousElement.su.firstOrNull?.id ?? 2);
bool isNotSeparated() => _parseWebuntisTimestamp(previousElement.date, previousElement.endTime).add(maxTimeBetweenDouble).add(const Duration(seconds: 1))
MineTec commented 2024-03-30 18:30:51 +00:00
Owner
Copy Link

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))`?
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 18:31:15 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -246,0 +263,4 @@
previousElement.endTime = currentElement.endTime;
timetableList.remove(currentElement);
i--;
}else{
MineTec commented 2024-03-30 18:31:15 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 18:35:19 +00:00
lib/view/pages/timetable/timetable.dart
@ -246,0 +259,4 @@
bool isNotSeparated() => _parseWebuntisTimestamp(previousElement.date, previousElement.endTime).add(maxTimeBetweenDouble).add(const Duration(seconds: 1))
.isAfter(_parseWebuntisTimestamp(currentElement.date, currentElement.startTime));
if(isSameSubject() && isNotSeparated()){
MineTec commented 2024-03-30 18:35:19 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 18:37:10 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -248,1 +276,3 @@
List<Appointment> appointments = data.getTimetableResponse.result.map((element) {
List<GetTimetableResponseObject> timetableList = data.getTimetableResponse.result.toList();
if(settings.val().timetableSettings.connectDoubleLessons){
MineTec commented 2024-03-30 18:37:09 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 18:41:22 +00:00
lib/view/settings/settings.dart
@ -100,1 +100,4 @@
ListTile(
leading: const Icon(Icons.calendar_view_day_outlined),
title: const Text("Doppelstunden als eine Stunde anzeigen"),
MineTec commented 2024-03-30 18:41:22 +00:00
Owner
Copy Link

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`?
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 18:59:26 +00:00
lib/view/settings/settings.dart
@ -101,0 +104,4 @@
trailing: Checkbox(
value: settings.val().timetableSettings.connectDoubleLessons,
onChanged: (e) {
settings.val(write: true).timetableSettings.connectDoubleLessons = e!;
MineTec commented 2024-03-30 18:59:26 +00:00
Owner
Copy Link

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:

lib/view/pages/timetable/timetable.dart Line 177 in 75846750f7
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)
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 19:06:38 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -246,0 +261,4 @@
if(isSameSubject() && isNotSeparated()){
previousElement.endTime = currentElement.endTime;
timetableList.remove(currentElement);
MineTec commented 2024-03-30 19:06:38 +00:00
Owner
Copy Link

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:

lib/view/pages/timetable/timetable.dart Line 327 in 75846750f7
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
Pupsi marked this conversation as resolved
MineTec commented 2024-03-30 19:10:02 +00:00
Owner
Copy Link

Dein committeter Stand ist älter als die aktuelle Version (ich habe zwischenzeitlich mehrere commits auf develop, welche bei dir noch nicht dabei sind)

du musst vor dem pr merge den aktuellen stand von develop in deinen branch reinmergen. Wenn du in deinem branch aktuell bist kannst du dies über git merge origin/devlop tun. Das geht in intellij auch über die oberfläche bei "develop" > "merge 'develop' into '...'"

in einer meiner letzten commits habe ich zudem die style guidelines des projektes angepasst. Doppelte anführungszeichen sollten überall durch einfache hochkommas ersetzt werden, lambdas sollten wenn möglich durch direkte methodenaufrufe ersetzt werden. Du kannst das auch automatisch anwenden mit dem befehl dart fix --apply

Ansonsten unterstrecht ab dann die IDE die entsprechenden stellen gelb.

Dein committeter Stand ist älter als die aktuelle Version (ich habe zwischenzeitlich mehrere commits auf develop, welche bei dir noch nicht dabei sind) du musst vor dem pr merge den aktuellen stand von develop in deinen branch reinmergen. Wenn du in deinem branch aktuell bist kannst du dies über `git merge origin/devlop` tun. Das geht in intellij auch über die oberfläche bei "develop" > "merge 'develop' into '...'" in einer meiner letzten commits habe ich zudem die style guidelines des projektes angepasst. Doppelte anführungszeichen sollten überall durch einfache hochkommas ersetzt werden, lambdas sollten wenn möglich durch direkte methodenaufrufe ersetzt werden. Du kannst das auch automatisch anwenden mit dem befehl `dart fix --apply` Ansonsten unterstrecht ab dann die IDE die entsprechenden stellen gelb.
Pupsi added 2 commits 2024-03-30 22:02:17 +00:00
Pupsi reviewed 2024-03-30 22:05:31 +00:00
Pupsi left a comment
Author
Member
Copy Link

Hab ich jetzt genug Leerzeichen hinzugefügt?
Und hat das mit dem Mergen alles geklappt?

Hab ich jetzt genug Leerzeichen hinzugefügt? Und hat das mit dem Mergen alles geklappt?
MineTec reviewed 2024-03-30 22:11:40 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -247,3 +298,3 @@
TimetableEvents _buildTableEvents(TimetableProps data) {
List<Appointment> appointments = data.getTimetableResponse.result.map((element) {
List<GetTimetableResponseObject> timetableList = data.getTimetableResponse.result.toList( );
MineTec commented 2024-03-30 22:11:40 +00:00
Owner
Copy Link

leerzeichen zu viel

leerzeichen zu viel
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 22:11:49 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -250,0 +300,4 @@
List<GetTimetableResponseObject> timetableList = data.getTimetableResponse.result.toList( );
if( settings.val().timetableSettings.connectDoubleLessons ){
timetableList = _removeDuplicates( data, const Duration(minutes: 5) );
MineTec commented 2024-03-30 22:11:49 +00:00
Owner
Copy Link

leerzeichen zu viel

leerzeichen zu viel
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 23:21:52 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -247,0 +248,4 @@
List<GetTimetableResponseObject> timetableList = data.getTimetableResponse.result.toList( );
if ( timetableList.isEmpty ) return timetableList;
MineTec commented 2024-03-30 23:21:52 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 23:22:04 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -247,0 +260,4 @@
int? currentSubjectId = currentElement.su.firstOrNull?.id;
int? previousSubjectId = previousElement.su.firstOrNull?.id;
if( currentSubjectId == null || previousSubjectId == null || currentSubjectId != previousSubjectId ) return false;
MineTec commented 2024-03-30 23:22:04 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 23:22:11 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -247,0 +265,4 @@
int? currentRoomId = currentElement.ro.firstOrNull?.id;
int? previousRoomId = previousElement.ro.firstOrNull?.id;
if( currentRoomId != previousRoomId ) return false;
MineTec commented 2024-03-30 23:22:11 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 23:22:21 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -247,0 +270,4 @@
int? currentTeacherId = currentElement.te.firstOrNull?.id;
int? previousTeacherId = previousElement.te.firstOrNull?.id;
if( currentTeacherId != previousTeacherId ) return false;
MineTec commented 2024-03-30 23:22:21 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 23:22:44 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -247,0 +283,4 @@
bool isNotSeparated() => _parseWebuntisTimestamp( previousElement.date, previousElement.endTime ).add( maxTimeBetweenDouble )
.isSameOrAfter( _parseWebuntisTimestamp( currentElement.date, currentElement.startTime ) );
if ( isSameLesson() && isNotSeparated() ) {
MineTec commented 2024-03-30 23:22:44 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 23:22:49 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -247,0 +285,4 @@
if ( isSameLesson() && isNotSeparated() ) {
previousElement.endTime = currentElement.endTime;
timetableList.remove( currentElement );
MineTec commented 2024-03-30 23:22:49 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 23:23:40 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -246,1 +246,4 @@
List<GetTimetableResponseObject> _removeDuplicates(TimetableProps data, Duration maxTimeBetweenDouble) {
List<GetTimetableResponseObject> timetableList = data.getTimetableResponse.result.toList( );
MineTec commented 2024-03-30 23:23:40 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 23:24:01 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -247,0 +280,4 @@
return true;
}
bool isNotSeparated() => _parseWebuntisTimestamp( previousElement.date, previousElement.endTime ).add( maxTimeBetweenDouble )
MineTec commented 2024-03-30 23:24:01 +00:00
Owner
Copy Link

leerzeichen zu viel nach pfeilsyntax

leerzeichen zu viel nach pfeilsyntax
MineTec commented 2024-03-30 23:24:22 +00:00
Owner
Copy Link

und weiter hinten ebenfalls

und weiter hinten ebenfalls
Pupsi marked this conversation as resolved
MineTec reviewed 2024-03-30 23:24:30 +00:00
lib/view/pages/timetable/timetable.dart Outdated
@ -247,0 +281,4 @@
}
bool isNotSeparated() => _parseWebuntisTimestamp( previousElement.date, previousElement.endTime ).add( maxTimeBetweenDouble )
.isSameOrAfter( _parseWebuntisTimestamp( currentElement.date, currentElement.startTime ) );
MineTec commented 2024-03-30 23:24:30 +00:00
Owner
Copy Link

leerzeichen

leerzeichen
Pupsi marked this conversation as resolved
MineTec commented 2024-03-30 23:27:42 +00:00
Owner
Copy Link

Nochmal zum merken:

  • vor und nach normalen klammern "(", ")" KEINE leerzeichen
  • vor öffnenden curly braces "{" immer ein leerzeichen gefolgt von einem Zeilenumbruch
  • vor schließenden curly braces "}" immer ein zeilenumbruch, gefolgt von einem leerzeichen
  • nach kommas ein leerzeichen

im zweifel an bestehendem code abschauen :)

Nochmal zum merken: - vor und nach normalen klammern "(", ")" KEINE leerzeichen - vor öffnenden curly braces "{" immer ein leerzeichen gefolgt von einem Zeilenumbruch - vor schließenden curly braces "}" immer ein zeilenumbruch, gefolgt von einem leerzeichen - nach kommas ein leerzeichen im zweifel an bestehendem code abschauen :)
MineTec commented 2024-03-30 23:31:58 +00:00
Owner
Copy Link

Hab ich jetzt genug Leerzeichen hinzugefügt?
Und hat das mit dem Mergen alles geklappt?

es haben eigentlich nur leerzeilen bei den curly braces gefehlt, ansonsten passt dein codestiel ja :)
orientier dich einfach an den bestehenden sachen

> Hab ich jetzt genug Leerzeichen hinzugefügt? > Und hat das mit dem Mergen alles geklappt? es haben eigentlich nur leerzeilen bei den curly braces gefehlt, ansonsten passt dein codestiel ja :) orientier dich einfach an den bestehenden sachen
Pupsi added 1 commit 2024-03-31 10:32:00 +00:00
MineTec merged commit 3e3e2579f0 into develop 2024-03-31 10:50:59 +00:00
MineTec deleted branch develop-connectedDoubleLessons 2024-03-31 10:50:59 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Something is not working
duplicate
This issue or pull request already exists
enhancement
New feature
feedback
Feedback von Nutzer
help wanted
Need some help
question
More information is needed
technnical
Not user facing
wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
bytedream goldbaja MineTec olischma Pupsi
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: MarianumMobile/Client#49
No description provided.
Delete Branch "develop-connectedDoubleLessons"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?