Skip to content

feat: add calendar grid to TimeRangePicker and fix custom date query #424

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.

Sign up for GitHub

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

Jump to bottom
Merged
iceljc merged 1 commit into from
Feb 3, 2026
Merged

feat: add calendar grid to TimeRangePicker and fix custom date query #424

iceljc merged 1 commit into from
Feb 3, 2026

Conversation

@yileicn
Copy link
Member

@yileicn yileicn commented Feb 3, 2026 •
edited by qodo-code-review bot
Loading

PR Type

Enhancement

Description

  • Add Flatpickr calendar grid component to custom date range picker

  • Implement temporary date state for custom date selection workflow

  • Refactor custom date confirmation logic with improved state management

  • Fix custom date query handling in convertTimeRange utility function

Diagram Walkthrough

flowchart LR
  A["TimeRangePicker Component"] -->|"Add Flatpickr"| B["Calendar Grid UI"]
  A -->|"Temp State"| C["tempStartDate/tempEndDate"]
  C -->|"Apply"| D["Update startDate/endDate"]
  D -->|"Dispatch Event"| E["Parent Component"]
  F["convertTimeRange Function"] -->|"Handle CUSTOM_DATE_RANGE"| G["Format Dates to UTC"]
Loading

File Walkthrough
Relevant files
Enhancement
TimeRangePicker.svelte
Integrate Flatpickr calendar and refactor date selection workflow

src/lib/common/shared/TimeRangePicker.svelte

  • Import and integrate Flatpickr library for inline calendar grid
    display
  • Add temporary date state variables (tempStartDate, tempEndDate) to
    manage custom date selection before confirmation
  • Implement initCustomDates() function to initialize calendar with
    existing or default dates
  • Add handleStartDateChange() and handleEndDateChange() functions to
    sync manual input with Flatpickr
  • Refactor handleCustomConfirm() to handleApply() with improved state
    management and explicit event dispatching
  • Update custom tab UI to display calendar grid and inline date input
    fields with "to" separator
  • Add global CSS rule to hide Flatpickr's default input field in inline
    mode
 +128/-23
Bug fix
common.js
Fix custom date range query handling in convertTimeRange 

src/lib/helpers/utils/common.js

  • Move CUSTOM_DATE_RANGE case handling to the beginning of
    convertTimeRange() function before checking TIME_RANGE_OPTIONS
  • Ensure custom date range queries are processed correctly with proper
    date validation and UTC formatting
  • Remove duplicate CUSTOM_DATE_RANGE case from switch statement to
    prevent unreachable code
 +15/-12 

@qodo-code-review
Copy link

qodo-code-review bot commented Feb 3, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

⚪
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing date validation: The new custom date workflow applies tempStartDate/tempEndDate without validating date
format or handling edge cases such as an end date earlier than the start date.

Referred Code 
function handleApply() {
	if (tempStartDate) {
		const finalEndDate = tempEndDate || tempStartDate;
		// Update props through binding (will trigger reactivity)
		startDate = tempStartDate;
		endDate = finalEndDate;
		timeRange = CUSTOM_DATE_RANGE;
		// Dispatch change event with updated values
		dispatch('change', { 
			timeRange: CUSTOM_DATE_RANGE, 
			startDate: tempStartDate, 
			endDate: finalEndDate 
		});
	}

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unvalidated user input: User-provided date strings from the new custom inputs are propagated into component state
and Flatpickr via setDate(...) without explicit validation/sanitization in the UI layer.

Referred Code 
// Handle manual input changes
function handleStartDateChange() {
	if (tempStartDate && flatpickrInstance) {
		if (tempEndDate) {
			flatpickrInstance.setDate([tempStartDate, tempEndDate], false);
		} else {
			flatpickrInstance.setDate([tempStartDate], false);
		}
	}
}

function handleEndDateChange() {
	if (tempStartDate && tempEndDate && flatpickrInstance) {
		flatpickrInstance.setDate([tempStartDate, tempEndDate], false);
	}
}

Learn more about managing compliance generic rules or creating your own custom rules
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Feb 3, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Correct UTC date conversion

Correct the moment.js usage in convertTimeRange to parse dates in UTC before
applying .startOf('day') or .endOf('day') to prevent timezone-related errors.

src/lib/helpers/utils/common.js [207-217]

 if (timeRange === CUSTOM_DATE_RANGE) {
     if (startDate && moment(startDate).isValid()) {
         const endDateToUse = endDate && moment(endDate).isValid() ? endDate : startDate;
         ret = {
             ...ret,
-            // @ts-ignore
-            startTime: moment(startDate).startOf('day').utc().format(),
-            // @ts-ignore
-            endTime: moment(endDateToUse).endOf('day').utc().format()
+            startTime: moment.utc(startDate).startOf('day').format(),
+            endTime: moment.utc(endDateToUse).endOf('day').format()
         };
     }
     return ret;
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: This suggestion corrects a subtle but critical bug in date handling, ensuring that date boundaries are calculated in UTC, which prevents timezone-related off-by-one-day errors.

Medium
Fix UI desync on date clear

Update the handleStartDateChange and handleEndDateChange functions to correctly
handle empty date inputs, ensuring the calendar UI remains synchronized when a
date is cleared.

src/lib/common/shared/TimeRangePicker.svelte [71-86]

     	// Handle manual input changes
     	function handleStartDateChange() {
-    		if (tempStartDate && flatpickrInstance) {
-    			if (tempEndDate) {
+    		if (flatpickrInstance) {
+    			if (tempStartDate) {
+    				const dates = [tempStartDate];
+    				if (tempEndDate) {
+    					dates.push(tempEndDate);
+    				}
+    				flatpickrInstance.setDate(dates, false);
+    			} else {
+    				// If start date is cleared, clear the selection and the end date
+    				flatpickrInstance.clear();
+    				tempEndDate = '';
+    			}
+    		}
+    	}
+
+    	function handleEndDateChange() {
+    		if (flatpickrInstance) {
+    			if (tempStartDate && tempEndDate) {
     				flatpickrInstance.setDate([tempStartDate, tempEndDate], false);
-    			} else {
+    			} else if (tempStartDate) {
+    				// If end date is cleared, but start date exists, select only start date
     				flatpickrInstance.setDate([tempStartDate], false);
     			}
     		}
     	}
 
-    	function handleEndDateChange() {
-    		if (tempStartDate && tempEndDate && flatpickrInstance) {
-    			flatpickrInstance.setDate([tempStartDate, tempEndDate], false);
-    		}
-    	}
-
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a UI bug where clearing date inputs desynchronizes them from the calendar, and the proposed fix correctly handles these edge cases, improving the component's robustness.

Medium
Handle invalid custom start date

Refactor the convertTimeRange function to use an early return if startDate is
invalid for a custom date range, improving code readability without changing its
logic.

src/lib/helpers/utils/common.js [199-224]

     export function convertTimeRange(timeRange, startDate, endDate) {
         let ret = { startTime: null, endTime: null };
 
         if (!timeRange) {
             return ret;
         }
 
         // Handle CUSTOM_DATE_RANGE first, as it's not in TIME_RANGE_OPTIONS
         if (timeRange === CUSTOM_DATE_RANGE) {
-            if (startDate && moment(startDate).isValid()) {
-                const endDateToUse = endDate && moment(endDate).isValid() ? endDate : startDate;
-                ret = {
-                    ...ret,
-                    // @ts-ignore
-                    startTime: moment(startDate).startOf('day').utc().format(),
-                    // @ts-ignore
-                    endTime: moment(endDateToUse).endOf('day').utc().format()
-                };
+            if (!startDate || !moment(startDate).isValid()) {
+                return ret;
             }
+            
+            const endDateToUse = endDate && moment(endDate).isValid() ? endDate : startDate;
+            ret = {
+                ...ret,
+                // @ts-ignore
+                startTime: moment(startDate).startOf('day').utc().format(),
+                // @ts-ignore
+                endTime: moment(endDateToUse).endOf('day').utc().format()
+            };
             return ret;
         }
 
         const found = TIME_RANGE_OPTIONS.find(x => x.value === timeRange);
         if (!found) {
             return ret;
         }
         ...
-```

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 2

__

Why: The proposed change is a stylistic refactoring to an early return pattern, which does not alter the function's behavior as the existing code already handles the described scenario correctly.

Low
High-level
Avoid unscoped global CSS selectors

The suggestion recommends scoping the :global(.flatpickr-input) CSS rule within
the TimeRangePicker.svelte component. This is to prevent potential styling
conflicts with other Flatpickr instances across the application.

Examples:

src/lib/common/shared/TimeRangePicker.svelte [331-333] 
	:global(.flatpickr-input) {
		display: none !important;
	}

Solution Walkthrough:

Before:

// In TimeRangePicker.svelte
...
<Flatpickr
  options={flatpickrOptions}
  ...
/>
...
<style>
  /* Hide flatpickr's default input field when using inline mode */
  :global(.flatpickr-input) {
    display: none !important;
  }
</style>

After:

// In TimeRangePicker.svelte
<div class="time-range-picker-wrapper">
  ...
  <Flatpickr
    options={flatpickrOptions}
    ...
  />
  ...
</div>
<style>
  /* Hide flatpickr's default input field inside this component */
  .time-range-picker-wrapper :global(.flatpickr-input) {
    display: none !important;
  }
</style>
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential for global CSS conflicts by using an unscoped :global() selector, which is a valid concern for component encapsulation and future maintainability.

Low
General
Add accessibility labels to inputs

Add elements and aria-label attributes to the start and end date inputs to
improve accessibility for screen readers.

src/lib/common/shared/TimeRangePicker.svelte [282-294]

+<label for="custom-start-date" class="form-label small mb-0">From</label>
 <Input
+    id="custom-start-date"
     type="date"
     bind:value={tempStartDate}
+    aria-label="Start date"
     class="form-control form-control-sm"
     on:change={handleStartDateChange}
 />
 ...
+<label for="custom-end-date" class="form-label small mb-0">To</label>
 <Input
+    id="custom-end-date"
     type="date"
     bind:value={tempEndDate}
+    aria-label="End date"
     class="form-control form-control-sm"
     on:change={handleEndDateChange}
 />

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion improves accessibility by adding labels for screen readers, which is a good practice, although it was likely present in the code removed during the refactoring.

Low
  • More

@iceljc iceljc merged commit 5741bb6 into SciSharp:main Feb 3, 2026
1 of 2 checks passed
@Ryan-Freya
Copy link

Ryan-Freya commented Feb 3, 2026

Reviewed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Review effort 2/5

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yileicn @Ryan-Freya @iceljc