Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience of the dashboard by introducing the ability to persist user selections for various filters and chart types. This means that once a user configures their preferred view, those settings will be automatically restored upon their next visit, eliminating the need for repetitive setup. Additionally, a new flexible date range option, 'Starting from weekday', has been added, providing more granular control over data visualization periods. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new "Starting from weekday" date range preset to the dashboard. This includes adding new UI elements for selecting the preset and a specific weekday, implementing JavaScript logic to manage the visibility of the weekday picker, and persisting the selected date preset and weekday to localStorage. The processData function was updated to correctly calculate the date range based on the chosen weekday. New unit tests were added to verify the functionality of this new preset. A suggestion was made to refactor the hardcoded localStorage keys into constants for improved maintainability and robustness.
| const savedPreset = localStorage.getItem('sp-dashboard-date-preset'); | ||
| if (savedPreset) { | ||
| presetSelect.value = savedPreset; | ||
| customContainer.classList.toggle('hidden', savedPreset !== 'custom'); | ||
| weekdayPickerContainer.classList.toggle('hidden', savedPreset !== 'from-weekday'); | ||
| const savedFrom = localStorage.getItem('sp-dashboard-date-from'); | ||
| const savedTo = localStorage.getItem('sp-dashboard-date-to'); | ||
| if (savedPreset === 'custom' && savedFrom) dateFromInput.value = savedFrom; | ||
| if (savedPreset === 'custom' && savedTo) dateToInput.value = savedTo; | ||
| const savedWeekday = localStorage.getItem('sp-dashboard-weekday-select'); | ||
| if (savedPreset === 'from-weekday' && savedWeekday) weekdaySelect.value = savedWeekday; | ||
| } | ||
|
|
||
| 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'); | ||
| localStorage.setItem('sp-dashboard-date-preset', e.target.value); | ||
| 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()); | ||
| document.getElementById('pie-chart-select').addEventListener('change', () => updatePieChart()); | ||
| weekdaySelect.addEventListener('change', () => { localStorage.setItem('sp-dashboard-weekday-select', weekdaySelect.value); processData(cachedTasks, cachedProjects); }); | ||
|
|
||
| dateFromInput.addEventListener('change', () => { localStorage.setItem('sp-dashboard-date-from', dateFromInput.value); processData(cachedTasks, cachedProjects); }); | ||
| dateToInput.addEventListener('change', () => { localStorage.setItem('sp-dashboard-date-to', dateToInput.value); processData(cachedTasks, cachedProjects); }); | ||
| const barChartSelect = document.getElementById('bar-chart-select'); | ||
| const pieChartSelect = document.getElementById('pie-chart-select'); | ||
|
|
||
| const savedBar = localStorage.getItem('sp-dashboard-bar-chart-select'); | ||
| const savedPie = localStorage.getItem('sp-dashboard-pie-chart-select'); | ||
| if (savedBar) barChartSelect.value = savedBar; | ||
| if (savedPie) pieChartSelect.value = savedPie; | ||
|
|
||
| barChartSelect.addEventListener('change', () => { localStorage.setItem('sp-dashboard-bar-chart-select', barChartSelect.value); updateBarChart(); }); | ||
| pieChartSelect.addEventListener('change', () => { localStorage.setItem('sp-dashboard-pie-chart-select', pieChartSelect.value); updatePieChart(); }); |
There was a problem hiding this comment.
The localStorage keys are currently hardcoded strings. It's a good practice to define these as constants (e.g., at the top of the script or in a dedicated constants object) to improve maintainability, prevent typos, and make it easier to manage them if they need to change in the future. This makes the code more robust and readable.
const LS_PREFIX = 'sp-dashboard-';
const LS_DATE_PRESET = LS_PREFIX + 'date-preset';
const LS_DATE_FROM = LS_PREFIX + 'date-from';
const LS_DATE_TO = LS_PREFIX + 'date-to';
const LS_WEEKDAY_SELECT = LS_PREFIX + 'weekday-select';
const LS_BAR_CHART_SELECT = LS_PREFIX + 'bar-chart-select';
const LS_PIE_CHART_SELECT = LS_PREFIX + 'pie-chart-select';
const savedPreset = localStorage.getItem(LS_DATE_PRESET);
if (savedPreset) {
presetSelect.value = savedPreset;
customContainer.classList.toggle('hidden', savedPreset !== 'custom');
weekdayPickerContainer.classList.toggle('hidden', savedPreset !== 'from-weekday');
const savedFrom = localStorage.getItem(LS_DATE_FROM);
const savedTo = localStorage.getItem(LS_DATE_TO);
if (savedPreset === 'custom' && savedFrom) dateFromInput.value = savedFrom;
if (savedPreset === 'custom' && savedTo) dateToInput.value = savedTo;
const savedWeekday = localStorage.getItem(LS_WEEKDAY_SELECT);
if (savedPreset === 'from-weekday' && savedWeekday) weekdaySelect.value = savedWeekday;
}
presetSelect.addEventListener('change', (e) => {
customContainer.classList.toggle('hidden', e.target.value !== 'custom');
weekdayPickerContainer.classList.toggle('hidden', e.target.value !== 'from-weekday');
localStorage.setItem(LS_DATE_PRESET, e.target.value);
processData(cachedTasks, cachedProjects);
});
weekdaySelect.addEventListener('change', () => { localStorage.setItem(LS_WEEKDAY_SELECT, weekdaySelect.value); processData(cachedTasks, cachedProjects); });
dateFromInput.addEventListener('change', () => { localStorage.setItem(LS_DATE_FROM, dateFromInput.value); processData(cachedTasks, cachedProjects); });
dateToInput.addEventListener('change', () => { localStorage.setItem(LS_DATE_TO, dateToInput.value); processData(cachedTasks, cachedProjects); });
const barChartSelect = document.getElementById('bar-chart-select');
const pieChartSelect = document.getElementById('pie-chart-select');
const savedBar = localStorage.getItem(LS_BAR_CHART_SELECT);
const savedPie = localStorage.getItem(LS_PIE_CHART_SELECT);
if (savedBar) barChartSelect.value = savedBar;
if (savedPie) pieChartSelect.value = savedPie;
barChartSelect.addEventListener('change', () => { localStorage.setItem(LS_BAR_CHART_SELECT, barChartSelect.value); updateBarChart(); });
pieChartSelect.addEventListener('change', () => { localStorage.setItem(LS_PIE_CHART_SELECT, pieChartSelect.value); updatePieChart(); });
Summary
Persists chart dropdown selections and date range preset across
sessions using
localStorage, so user preferences are restored onnext load.
Changes
index.html— save/restore bar chart, pie chart, date preset,custom date range, and weekday picker selections
Observe that here I am pulling two features, the one already present in #10 and the described one.