-
Notifications
You must be signed in to change notification settings - Fork 1
Date filter #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Date filter #178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the Date Range Slider component to fix bugs and allow users to set minimum and maximum date ranges. The changes replace the previous date formatting with interactive DatePicker components and add state management for better user control.
Key changes:
- Replaced static date labels with interactive DatePicker components in both the main products page and date range slider
- Added state synchronization between parent and child components for date range management
- Implemented ResizeObserver for better responsive behavior and layout calculations
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app-pages/products/products.js | Added DatePicker components to main interface with state management for start/end dates |
| src/app-pages/products/date-range-slider.js | Refactored slider to use DatePicker components and improved layout calculations with ResizeObserver |
| src/app-pages/products/DatePickerOverrides.css | Added custom CSS styling for DatePicker components |
| src/app-bundles/product-bundle.js | Added fallback logic for date range boundaries |
| package.json | Updated react-datepicker dependency and added react-date-picker and react-calendar |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| calendarIcon={<CalendarIcon className="w-5 h-5 ml-[-10px]"/>} | ||
| calendarProps={{ | ||
| prevLabel: <ChevronLeftIcon/>, | ||
| nextLabel: <ChevronRightIcon/>, |
Copilot
AI
Sep 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace after the comma on these lines should be removed.
| nextLabel: <ChevronRightIcon/>, | |
| nextLabel: <ChevronRightIcon/>, |
| calendarIcon={<CalendarIcon className="w-5 h-5 ml-[-10px]"/>} | ||
| calendarProps={{ | ||
| prevLabel: <ChevronLeftIcon/>, | ||
| nextLabel: <ChevronRightIcon/>, |
Copilot
AI
Sep 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace after the comma on these lines should be removed.
| calendarIcon={<CalendarIcon className="w-5 h-5 ml-[-10px]"/>} | ||
| calendarProps={{ | ||
| prevLabel: <ChevronLeftIcon/>, | ||
| nextLabel: <ChevronRightIcon/>, |
Copilot
AI
Sep 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace after the comma on these lines should be removed.
| nextLabel: <ChevronRightIcon/>, | |
| nextLabel: <ChevronRightIcon/>, |
src/app-pages/products/products.js
Outdated
| calendarIcon={<CalendarIcon className="w-5 h-5"/>} | ||
| calendarProps={{ | ||
| prevLabel: <ChevronLeftIcon/>, | ||
| nextLabel: <ChevronRightIcon/>, |
Copilot
AI
Sep 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace after the comma on these lines should be removed.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: define a shared invalid_date (or whatever jan 1, 3000 means)
invalid_date = new Date('01-Jan-3000')
and use everywhere instead of magic date.
src/app-bundles/product-bundle.js
Outdated
| } | ||
| }); | ||
| if (out.getTime() === new Date('01-Jan-3000').getTime()) { | ||
| out = new Date('01-Jan-1980'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider defining
cumulus_begin = new Date('01-Jan-1980')
and use that wherever used.
| if (barRef.current) { | ||
| barWidth = barRef.current.offsetWidth; | ||
| } | ||
| const [offsetFrom, setOffsetFrom] = useState(((from - min) / (max - min)) * (barWidth - labelWidthFrom)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const [offsetFrom, setOffsetFrom] = useState(((from - min) / (max - min)) * (barWidth - labelWidthFrom)); | |
| const posPx = ( value, min, max, containerPx, subtractPx = 0, addPx = 0) => | |
| ((value - min) / (max - min)) * (containerPx - subtractPx) + addPx; | |
| const [offsetFrom, setOffsetFrom] = useState(posPx(from,min,max,barWidth,labelWidthFrom)); | |
| const [offsetTo, setOffsetTo] = useState(posPx(to, min, max, barWidth, labelWidthTo)); | |
| const [rangeBarFrom, setRangeBarFrom] = useState(posPx(from, min, max, barWidth, 14, 7) ); | |
| const [rangeBarTo, setRangeBarTo] = useState( posPx(to, min, max, barWidth, 14, 7)); | |
| useEffect(() => { | ||
| setOffsetFrom(((from - min) / (max - min)) * (barWidth - labelWidthFrom)); | ||
| setOffsetTo(((to - min) / (max - min)) * (barWidth - labelWidthTo)); | ||
| setRangeBarFrom(((from - min) / (max - min)) * (barWidth - 14) + 7); | ||
| setRangeBarTo(((to - min) / (max - min)) * (barWidth - 14) + 7); | ||
| }, [barWidth, labelWidthFrom, labelWidthTo, min, max, from, to]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| useEffect(() => { | |
| setOffsetFrom(((from - min) / (max - min)) * (barWidth - labelWidthFrom)); | |
| setOffsetTo(((to - min) / (max - min)) * (barWidth - labelWidthTo)); | |
| setRangeBarFrom(((from - min) / (max - min)) * (barWidth - 14) + 7); | |
| setRangeBarTo(((to - min) / (max - min)) * (barWidth - 14) + 7); | |
| }, [barWidth, labelWidthFrom, labelWidthTo, min, max, from, to]); | |
| useEffect(() => { | |
| setOffsetFrom (posPx(from, min, max, barWidth, labelWidthFrom)); | |
| setOffsetTo (posPx(to, min, max, barWidth, labelWidthTo)); | |
| setRangeBarFrom(posPx(from, min, max, barWidth, 14, 7)); | |
| setRangeBarTo (posPx(to, min, max, barWidth, 14, 7)); | |
| }, [barWidth, labelWidthFrom, labelWidthTo, min, max, from, to]); | |
ktarbet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please consider making:
const MAX_DATE = new Date('3000-01-01'); (or better name if you have one)
for clarity, instead of magic number.
Update the Date Range Slider to fix bugs and allow the user to set a min and max range.