Fix #105: Implement time-based separators in agenda view#133
Fix #105: Implement time-based separators in agenda view#133lambeletjp wants to merge 1 commit intoagilare:masterfrom
Conversation
lambeletjp
commented
Dec 30, 2025
lambeletjp
commented
- Add EvenementWithSeparatorCollection to handle separator logic
- Add EventWithSeparator wrapper class with shouldDisplaySeparator() method
- Add Separator class to generate separator labels
- Refactor index.php template to use new collection classes
- Add test data SQL file for time-based separators
- Add EvenementWithSeparatorCollection to handle separator logic - Add EventWithSeparator wrapper class with shouldDisplaySeparator() method - Add Separator class to generate separator labels - Refactor index.php template to use new collection classes - Add test data SQL file for time-based separators
agilare
left a comment
agilare
left a comment
There was a problem hiding this comment.
Thanks for this proposition, I made some suggestions in the code
Personnaly I would use less classes and variables but it should be also my oldschool procedural style
I did a try with some events and I reveal a problem causing confusion : the hours in separators could mean it the hours of the following event, if we arn't enough attentive
We should find a disambiguation for ex. dès 20h..." instead of only "20h"
or even a separator for each hour change
| } | ||
| } | ||
|
|
||
| private function calculateEvenHour(string $horaire_debut): int |
There was a problem hiding this comment.
The function name could be clearer informing we get the previous even hour
A short DocBlock with an example could also help
| $this->date_next_day = date_lendemain($date_current); | ||
| } | ||
|
|
||
| public function getIterator(): \Traversable |
There was a problem hiding this comment.
A synthesis of the process and the intention in a DocBlock would be appreciated
Variables with "no_time" and "autres_horaires" should have their name harmonized and more accurate; all mean there is no horaire debut defined
I wondered if Separator instances could be created here and injected in EventWithSeparator constructor instead of EventWithSeparator ::getCurrentSeparator()
There was a problem hiding this comment.
Some comments explaining the contexte of use of this class could be useful
3 participants
