fix: Click on schedule card redirects to next lecture's day #1373
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1373 +/- ##
=======================================
+ Coverage 12% 12% +1%
=======================================
Files 266 266
Lines 7201 7212 +11
=======================================
+ Hits 803 811 +8
- Misses 6398 6401 +3 |
| widget.currentWeek.weekdays.take(6).forEach((day) { | ||
| final lectures = lecturesOfDay(widget.lectures, day); | ||
| lecturesThisWeek.addAll(lectures); | ||
| }); |
There was a problem hiding this comment.
Since this code is repeated, I'll propose a refactoring to help cleanup the code.
To be continued...
There was a problem hiding this comment.
You can store the lecturesThisWeek at the state level (by creating a property late final List<Lecture> lecturesThisWeek) and setting it in the initState method.
Then, below, you can simply access that array instead of filtering again.
There was a problem hiding this comment.
The repeated code actually does slightly different things. The first one is to get the lectures of the week (using forEach) and the other to map the days of the week to the lectures of each day (using map).
I made changes to the build function to look for the lectures of the day on the lecturesThisWeek list instead of going through the full list of lectures of the semester for each day of the week.
Another alternative could be to initially store lecturesThisWeek in a Map and the build code can iterate only once over this map to build the widget list. What do you think?
| if (lecturesThisWeek.isNotEmpty) { | ||
| final now = DateTime.now(); | ||
| Lecture? nextLecture; | ||
| Duration? closestDuration; | ||
|
|
||
| for (final lecture in lecturesThisWeek) { | ||
| if (lecture.endTime.isAfter(now)) { | ||
| final difference = lecture.endTime.difference(now); | ||
| if (closestDuration == null || difference < closestDuration) { | ||
| closestDuration = difference; | ||
| nextLecture = lecture; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This can be changed for something like final nextLecture = lecturesThisWeek.firstWhere((lecture) => lecture.endTime.isAfter(widget.now));
I am assuming that there are no concurrent lectures and that they are ordered in the list (I think they are)
There was a problem hiding this comment.
lecturesThisWeek isn't in chronological order, it always starts on Monday and ends on Saturday even if the next class is on a Tuesday.
I made changes to use where and fold functions instead of this to make the code cleaner.
athoscf
changed the title
Closes #1359
Clicking on home schedule card redirects to next lecture's day instead of current day.
Changes were made to the
initStateof the schedule so opening the schedule from anywhere starts at the next lecture's day.If there are no classes in the next week it opens the current day.
Review checklist
whatsnew/whatsnew-pt-PTchangelog.mdwith the change