feat: Add past weeks/months for Autofill/Clear/Export windows #3428
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe pull request introduces a new environment variable Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/features/meetings/week_range_selector/index.tsx (1)
Line range hint
41-58: Enhance accessibility for date selection.The date selection UI could benefit from improved accessibility features.
Add ARIA labels and group roles:
+<div role="group" aria-label={t('tr_pastDates')}> <MenuSubHeader>{t('tr_pastDates')}</MenuSubHeader> {startPastWeeks.map((option) => ( <MenuItem key={option.value} value={option.value} + aria-label={`${t('tr_startWeek')} ${option.label}`} > <Typography className="body-regular" color="var(--black)"> {option.label} </Typography> </MenuItem> ))} +</div>
🧹 Nitpick comments (4)
src/components/menu_sub_header/index.tsx (1)
1-19: Consider enhancing component flexibility with additional props.The component is well-structured, but could benefit from additional customization options:
- Allow overriding styles through props
- Add optional padding/margin props
- Consider adding a prop for sticky positioning
Example enhancement:
-export const MenuSubHeader = ({ children }: PropsWithChildren) => { +type MenuSubHeaderProps = PropsWithChildren<{ + className?: string; + sticky?: boolean; + sx?: Record<string, unknown>; +}>; + +export const MenuSubHeader = ({ + children, + className = 'body-small-semibold', + sticky = false, + sx = {}, +}: MenuSubHeaderProps) => { return ( <ListSubheader - className="body-small-semibold" + className={className} sx={{ color: 'var(--accent-dark)', font: 'inherit', padding: '16px 16px 8px 16px', userSelect: 'none', backgroundColor: 'var(--white)', + position: sticky ? 'sticky' : 'static', + ...sx, }} >src/features/meetings/week_range_selector/useWeekRangeSelector.tsx (1)
33-46: Consider memoizing mapSourcesToOptions function.The mapping function could benefit from memoization to prevent unnecessary recalculations.
+const memoizedMapSourcesToOptions = useMemo(() => (sourceList: typeof sources) => sourceList.map((source) => { const [year, month, date] = source.weekOf.split('/'); const monthName = monthNames[+month - 1]; return { label: t('tr_longDateWithYearLocale', { year, month: monthName, date, }), value: source.weekOf, }; }); +, [monthNames, t]);src/features/meetings/schedule_range_selector/useScheduleRangeSelector.tsx (1)
25-40: Optimize date object creation in filtering function.The current implementation creates new Date objects for each comparison. Consider caching the date object creation:
const filterAndSortSources = ( sources: SourceWeekType[], startDate: Date, endDate: Date ) => { + const sourceCache = sources.map(source => ({ + ...source, + date: new Date(source.weekOf) + })); return sources .filter( (source) => - new Date(source.weekOf) >= startDate && - new Date(source.weekOf) < endDate && + source.date >= startDate && + source.date < endDate && source.midweek_meeting.week_date_locale[lang] ) .sort( - (a, b) => new Date(a.weekOf).getTime() - new Date(b.weekOf).getTime() + (a, b) => a.date.getTime() - b.date.getTime() ); };src/utils/date.ts (1)
6-6: Consider a more descriptive name for MAX_DATE constant.The constant could benefit from a more descriptive name and documentation explaining its purpose.
-export const MAX_DATE = new Date(9999, 11, 31); +/** Maximum allowed date for date range operations (December 31, 9999) */ +export const MAXIMUM_SUPPORTED_DATE = new Date(9999, 11, 31);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/locales/en/meetings.jsonis excluded by!**/*.json
📒 Files selected for processing (9)
.env.example(1 hunks)LOCAL_ENVIRONMENT_SETUP.md(1 hunks)src/components/menu_sub_header/index.tsx(1 hunks)src/features/meetings/schedule_range_selector/index.tsx(4 hunks)src/features/meetings/schedule_range_selector/useScheduleRangeSelector.tsx(2 hunks)src/features/meetings/week_range_selector/index.tsx(4 hunks)src/features/meetings/week_range_selector/useWeekRangeSelector.tsx(2 hunks)src/features/reports/field_service/selector_stats/person_filter/index.tsx(1 hunks)src/utils/date.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .env.example
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (5)
src/features/reports/field_service/selector_stats/person_filter/index.tsx (1)
6-6: LGTM! Good component reuse.The replacement of the local MenuSubHeader with the shared component improves code reusability and maintains consistency across the application.
src/features/meetings/schedule_range_selector/index.tsx (1)
4-4: LGTM! UI improvements enhance user experience.The changes effectively organize month options into "Past Dates" and "Upcoming Dates" categories using the
MenuSubHeadercomponent, making the selection interface more intuitive and user-friendly.Also applies to: 20-20, 22-22, 40-41, 49-50
src/features/meetings/schedule_range_selector/useScheduleRangeSelector.tsx (1)
25-40: LGTM! Well-organized code with clear separation of concerns.The extraction of filtering and sorting logic into a separate function improves code maintainability and reusability.
src/utils/date.ts (1)
15-29: LGTM! Well-structured date utility functions.The new utility functions are well-organized, follow consistent naming patterns, and maintain single responsibility principle.
LOCAL_ENVIRONMENT_SETUP.md (1)
180-180: LGTM! Clear documentation for DEMO MODE feature.The documentation clearly explains the purpose and configuration of the DEMO MODE feature, helping developers understand its usage during testing and demonstration.
| const filterSources = (startDate: Date, endDate: Date) => | ||
| sources.filter( | ||
| (source) => | ||
| new Date(source.weekOf) >= startDate && | ||
| new Date(source.weekOf) < endDate && | ||
| ((meeting === 'midweek' && | ||
| source.midweek_meeting.week_date_locale[lang]) || | ||
| meeting === 'weekend') | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance date comparison robustness.
The date comparison logic could be improved to handle edge cases and timezone issues.
Consider this enhancement:
const filterSources = (startDate: Date, endDate: Date) =>
sources.filter(
(source) =>
- new Date(source.weekOf) >= startDate &&
- new Date(source.weekOf) < endDate &&
+ new Date(source.weekOf).setHours(0, 0, 0, 0) >= startDate.setHours(0, 0, 0, 0) &&
+ new Date(source.weekOf).setHours(0, 0, 0, 0) < endDate.setHours(0, 0, 0, 0) &&
((meeting === 'midweek' &&
source.midweek_meeting.week_date_locale[lang]) ||
meeting === 'weekend')Committable suggestion skipped: line range outside the PR's diff.
ild0tt0re
force-pushed
the
feat/add-past-weeks-months
branch
from
122219f to
4b02050
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/features/meetings/week_range_selector/useWeekRangeSelector.tsx (1)
23-31: 🛠️ Refactor suggestionImprove date comparison robustness.
The date comparison logic needs enhancement to handle timezone-related edge cases.
Apply this diff to normalize the dates for comparison:
const filterSources = (startDate: Date, endDate: Date) => sources.filter( (source) => - new Date(source.weekOf) >= startDate && - new Date(source.weekOf) < endDate && + new Date(source.weekOf).setHours(0, 0, 0, 0) >= startDate.setHours(0, 0, 0, 0) && + new Date(source.weekOf).setHours(0, 0, 0, 0) < endDate.setHours(0, 0, 0, 0) && ((meeting === 'midweek' && source.midweek_meeting.week_date_locale[lang]) || meeting === 'weekend') );
🧹 Nitpick comments (3)
src/features/meetings/week_range_selector/useWeekRangeSelector.tsx (1)
33-46: Add error handling for date parsing.The date splitting operation assumes a valid date format. Consider adding error handling for malformed dates.
const mapSourcesToOptions = (sourceList: typeof sources) => sourceList.map((source) => { + try { const [year, month, date] = source.weekOf.split('/'); + if (!year || !month || !date) { + throw new Error('Invalid date format'); + } const monthName = monthNames[+month - 1]; + if (!monthName) { + throw new Error('Invalid month'); + } return { label: t('tr_longDateWithYearLocale', { year, month: monthName, date, }), value: source.weekOf, }; + } catch (error) { + console.error(`Invalid date format for source: ${source.weekOf}`); + return null; + } }).filter(Boolean);src/features/meetings/schedule_range_selector/useScheduleRangeSelector.tsx (2)
42-58: Consider memoizing month names lookup.The
generateScheduleOptionsfunction accessesmonthNamesarray in a loop. Consider memoizing the month name lookup to avoid potential performance issues with large datasets.const generateScheduleOptions = (sources: SourceWeekType[]) => { const result: ScheduleOptionsType[] = []; + const monthNameMap = new Map(monthNames.map((name, i) => [i + 1, name])); for (const source of sources) { const [year, month] = source.weekOf.split('/'); const isExist = result.find( (schedule) => schedule.value === `${year}/${month}` ); if (!isExist) { - const monthName = monthNames[+month - 1]; + const monthName = monthNameMap.get(+month); result.push({ label: `${monthName} ${year}`, value: `${year}/${month}`, }); } } return result; };
78-83: Consider adding type safety for array destructuring.The endMonthOptions logic assumes startMonthOptions always returns a tuple of two arrays. Consider adding type safety to handle potential edge cases.
- if (startMonth.length <= 0) return [[], []]; + if (startMonth.length <= 0) return [[], []] as const; - const endSchedules = startMonthOptions.map((options) => { + const [pastOptions, upcomingOptions] = startMonthOptions; + const endSchedules = [ + pastOptions.filter((schedule) => schedule.value >= startMonth), + upcomingOptions.filter((schedule) => schedule.value >= startMonth), + ] as const; - return options.filter((schedule) => schedule.value >= startMonth); - }); return endSchedules;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/locales/en/meetings.jsonis excluded by!**/*.json
📒 Files selected for processing (9)
.env.example(1 hunks)LOCAL_ENVIRONMENT_SETUP.md(1 hunks)src/components/menu_sub_header/index.tsx(1 hunks)src/features/meetings/schedule_range_selector/index.tsx(4 hunks)src/features/meetings/schedule_range_selector/useScheduleRangeSelector.tsx(2 hunks)src/features/meetings/week_range_selector/index.tsx(4 hunks)src/features/meetings/week_range_selector/useWeekRangeSelector.tsx(2 hunks)src/features/reports/field_service/selector_stats/person_filter/index.tsx(1 hunks)src/utils/date.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- .env.example
- LOCAL_ENVIRONMENT_SETUP.md
- src/features/reports/field_service/selector_stats/person_filter/index.tsx
- src/components/menu_sub_header/index.tsx
- src/utils/date.ts
- src/features/meetings/week_range_selector/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (9)
src/features/meetings/week_range_selector/useWeekRangeSelector.tsx (3)
3-3: LGTM! Import statements are well-organized.The new imports are properly structured and align with the added functionality.
Also applies to: 5-5
48-53: LGTM! Clear separation of past and upcoming sources.The implementation effectively separates past and upcoming sources using appropriate date utility functions.
55-58: LGTM! Well-structured return values.The separation of past and upcoming options in both
startWeekOptionsandendWeekOptionsis well-implemented and maintains consistency.Also applies to: 62-69
src/features/meetings/schedule_range_selector/index.tsx (3)
4-4: LGTM! Clean import organization.The imports are well-organized, with related components grouped together.
Also applies to: 8-9
20-20: LGTM! Clear state destructuring.The destructuring pattern effectively splits the options into past and upcoming months, improving code readability.
Also applies to: 22-22
40-50: LGTM! Well-structured UI with clear date categorization.The implementation effectively separates past and upcoming dates using MenuSubHeader, providing clear visual hierarchy and improved user experience. The structure is consistently applied to both start and end month selectors.
Also applies to: 70-80
src/features/meetings/schedule_range_selector/useScheduleRangeSelector.tsx (3)
4-9: LGTM! Well-organized date utility imports.The date utility functions are appropriately imported and provide clear functionality for date range handling.
60-74: LGTM! Clear separation of past and recent sources.The implementation effectively separates and processes past and recent sources using appropriate date utility functions.
25-40: Consider adding error handling for invalid dates.The
filterAndSortSourcesfunction should handle potential invalid dates in the source data.Consider adding date validation:
const filterAndSortSources = ( sources: SourceWeekType[], startDate: Date, endDate: Date ) => { return sources .filter( - (source) => - new Date(source.weekOf) >= startDate && - new Date(source.weekOf) < endDate && - source.midweek_meeting.week_date_locale[lang] + (source) => { + try { + const sourceDate = new Date(source.weekOf); + return sourceDate >= startDate && + sourceDate < endDate && + source.midweek_meeting.week_date_locale[lang]; + } catch { + console.warn(`Invalid date found: ${source.weekOf}`); + return false; + } + } )
Description
[FEAT] Add past weeks/months for Autofill/Clear/Export windows
task: https://app.clickup.com/t/86c1kj9fg
Type of change
Please delete options that are not relevant.
Checklist: