Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 23 additions & 6 deletions sp-dashboard/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,19 @@ <h1 class="title">Dashboard</h1>
<option value="month">Past Month</option>
<option value="year">Past Year</option>
<option value="custom">Custom Range...</option>
<option value="from-weekday">Starting from weekday</option>
</select>
</div>
<div id="weekday-picker-container" class="control-box hidden">
<label for="weekday-select">Weekday</label>
<select id="weekday-select">
<option value="1">Monday</option>
<option value="2">Tuesday</option>
<option value="3">Wednesday</option>
<option value="4">Thursday</option>
<option value="5">Friday</option>
<option value="6">Saturday</option>
<option value="0">Sunday</option>
</select>
</div>
</div>
Expand Down Expand Up @@ -517,16 +530,16 @@ <h3 class="card-title">Project Breakdown</h3>

const presetSelect = document.getElementById('date-preset');
const customContainer = document.getElementById('custom-date-container');
const weekdayPickerContainer = document.getElementById('weekday-picker-container');

presetSelect.addEventListener('change', (e) => {
if (e.target.value === 'custom') {
customContainer.classList.remove('hidden');
} else {
customContainer.classList.add('hidden');
}
customContainer.classList.toggle('hidden', e.target.value !== 'custom');
weekdayPickerContainer.classList.toggle('hidden', e.target.value !== 'from-weekday');
processData(cachedTasks, cachedProjects);
});

document.getElementById('weekday-select').addEventListener('change', () => processData(cachedTasks, cachedProjects));

document.getElementById('date-from').addEventListener('change', () => processData(cachedTasks, cachedProjects));
document.getElementById('date-to').addEventListener('change', () => processData(cachedTasks, cachedProjects));
document.getElementById('bar-chart-select').addEventListener('change', () => updateBarChart());
Expand Down Expand Up @@ -763,6 +776,10 @@ <h3 class="card-title">Project Breakdown</h3>
startObj.setMonth(endObj.getMonth() - 1);
} else if (preset === 'year') {
startObj.setFullYear(endObj.getFullYear() - 1);
} else if (preset === 'from-weekday') {
const targetDay = parseInt(document.getElementById('weekday-select').value, 10);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better performance and maintainability, it's a good practice to cache DOM element references instead of querying for them repeatedly. The weekday-select element is accessed here and also when attaching an event listener (line 541). You can get the element once and store it in a variable at the top of the script block, similar to how presetSelect and customContainer are handled.

For example, you could add this line near line 533:

const weekdaySelect = document.getElementById('weekday-select');

Then you would change line 541 to:

weekdaySelect.addEventListener('change', () => processData(cachedTasks, cachedProjects));

And this line would become:

Suggested change
const targetDay = parseInt(document.getElementById('weekday-select').value, 10);
const targetDay = parseInt(weekdaySelect.value, 10);

const daysBack = (endObj.getDay() - targetDay + 7) % 7;
startObj.setDate(endObj.getDate() - daysBack);
}
dateFromStr = startObj.toISOString().split('T')[0];
dateToStr = endObj.toISOString().split('T')[0];
Expand Down Expand Up @@ -1108,4 +1125,4 @@ <h3 class="card-title">Project Breakdown</h3>

</script>
</body>
</html>
</html>
27 changes: 27 additions & 0 deletions tests/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,33 @@ describe('Date Range Reporter UI', () => {
expect(pieLegend.querySelector('.legend-item')).not.toBeNull();
});

it('from-weekday preset with weekday picker should produce correct date range', () => {
const presetSelect = document.getElementById('date-preset');
const weekdaySelect = document.getElementById('weekday-select');
const barContainer = document.getElementById('bar-chart-container');
const weekdayPickerContainer = document.getElementById('weekday-picker-container');

presetSelect.value = 'from-weekday';
presetSelect.dispatchEvent(new Event('change'));
expect(weekdayPickerContainer.classList.contains('hidden')).toBe(false);

// Test each weekday
const today = new Date();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This test could be flaky because it relies on the current system time (new Date()). If the test execution crosses a day boundary between this line and the new Date() call inside processData, the test might fail unexpectedly. It's a best practice to mock date/time for such tests to ensure they are deterministic.

You can use vi.useFakeTimers() and vi.setSystemTime() to control the system time within your test. For example:

const mockToday = new Date('2024-01-26T12:00:00Z'); // A Friday
vi.useFakeTimers();
vi.setSystemTime(mockToday);

// ... your test logic ...

vi.useRealTimers(); // cleanup

for (const targetDay of [0, 1, 2, 3, 4, 5, 6]) {
weekdaySelect.value = String(targetDay);
weekdaySelect.dispatchEvent(new Event('change'));
window.processData([], []);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The dispatchEvent on the previous line already triggers processData via the event listener. This explicit call to window.processData is redundant and can be removed.


const daysBack = (today.getDay() - targetDay + 7) % 7;
expect(barContainer.querySelectorAll('.bar-col').length).toBe(daysBack + 1);
}

// Switching away hides the picker
presetSelect.value = 'today';
presetSelect.dispatchEvent(new Event('change'));
expect(weekdayPickerContainer.classList.contains('hidden')).toBe(true);
});

it('detail list columns are sortable when headers are clicked', () => {
// create two tasks with different dates
const taskA = { id:'a', parentId:null, title:'A', isDone:false, dueDay:'2026-01-01', timeSpentOnDay:{'2026-01-01':3600000} };
Expand Down