Components for DateRange filter#103
Conversation
| dateFrom: string | undefined | ||
| dateTo: string | undefined | ||
| emptyDateFromLabel: string | ||
| emptyDateToLabel: string |
There was a problem hiding this comment.
Ik zou liever een Date | undefined doorgeven maar dan moeten we de formatting hier doen. Het formatten geef ik liever niet uit handen dus daarom heb ik het toch maar zo gelaten.
There was a problem hiding this comment.
Lijkt me dat schermen, locale-afhankelijke (date)formattering en i18n beter in de apps kan blijven. Wellicht dat we voor het formatteren van dates een useLocale()/LocaleProvider kunnen toevoegen, analoog aan hoe we theme doen, maar dat levert niet heel veel op. Dus prima zo.
| dateFrom: string | undefined | ||
| dateTo: string | undefined | ||
| emptyDateFromLabel: string | ||
| emptyDateToLabel: string |
There was a problem hiding this comment.
Lijkt me dat schermen, locale-afhankelijke (date)formattering en i18n beter in de apps kan blijven. Wellicht dat we voor het formatteren van dates een useLocale()/LocaleProvider kunnen toevoegen, analoog aan hoe we theme doen, maar dat levert niet heel veel op. Dus prima zo.
| </View> | ||
| </TouchableOpacity> | ||
|
|
||
| <View style={styles.divider}> |
There was a problem hiding this comment.
Foutje overgenomen van Observation: deze style had op de Text gemoeten, een color style op een View doet niets.
| } | ||
|
|
||
| const DatePicker = ({ | ||
| selectedDate, |
There was a problem hiding this comment.
Claude moppert wat over selectedDate die momenteel buiten de range minimumDate tot maximumDate kan liggen. We hebben daar geen unit test voor. Claude stelt voor om selectedDate te clampen op een geldige date in de range, en om dat in deze component te doen, niet in het scherm.
Zonder hier serieus in te duiken kan ik niet overzien of dit in de praktijk ook wat betekent. Kun jij checken of hier wat te verbeteren valt?
2 participants
Adding the components that are used in the FromToDateFilter.