Add full-width date bar with dynamic timer above model panes#1
Add full-width date bar with dynamic timer above model panes#1dominiksipowicz wants to merge 1 commit intomainfrom
Conversation
Introduce full-width date bar and timer for dynamic date update Co-authored-by: Dom Sip <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR adds a date bar component above the AI service panes to display the current date, with a timer mechanism to keep it updated. The changes modify the dashboard layout to accommodate the new date bar and implement dynamic date updates.
Changes:
- Added a full-width date bar component that displays the current date in long format (e.g., "Monday, January 15, 2024")
- Implemented a 60-second interval timer to update the date display dynamically
- Restructured the iframe container layout to support the date bar above the service panes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| dashboard/dashboard.js | Added date formatting function, timer mechanism, and date bar DOM creation; restructured iframe rendering to include date bar and panes row wrapper |
| dashboard/dashboard.css | Added date bar styling, modified iframe-container to use flex-wrap, and created new iframe-panes-row wrapper styles |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| .iframe-container { | ||
| display: flex; | ||
| flex-wrap: wrap; |
There was a problem hiding this comment.
Adding flex-wrap: wrap to the iframe-container changes the flex behavior. Since all children (date-bar and iframe-panes-row) are expected to stay in a column layout, consider using flex-direction: column instead of flex-wrap: wrap. This would be more semantically correct for a vertical layout and avoid potential unwanted wrapping behavior.
| flex-wrap: wrap; | |
| flex-direction: column; |
| if (enabledKeys.length === 0) { | ||
| container.innerHTML = ` | ||
| <div class="empty-state"> | ||
| container.innerHTML = ''; | ||
| container.appendChild(dateBar); |
There was a problem hiding this comment.
The date bar is created twice in the empty state path. First it's created and appended at line 180-184, then the container innerHTML is cleared at line 190, and the date bar is recreated and appended again at line 191. This is inefficient. Instead, save a reference to the date bar element before clearing innerHTML, or restructure the code to avoid recreating it.
|
|
||
| function startDateTimer() { | ||
| if (dateTimerId) clearInterval(dateTimerId); | ||
| // Check every minute if the date has changed | ||
| dateTimerId = setInterval(() => { | ||
| const dateBar = document.getElementById('dateBar'); | ||
| if (dateBar) { | ||
| dateBar.textContent = formatCurrentDate(); | ||
| } |
There was a problem hiding this comment.
The timer updates the date bar every minute regardless of whether the date has actually changed. This is wasteful since the date only changes once per day. Consider checking if the date has actually changed before updating the DOM, or adjust the timer logic to only update when crossing midnight.
| function startDateTimer() { | |
| if (dateTimerId) clearInterval(dateTimerId); | |
| // Check every minute if the date has changed | |
| dateTimerId = setInterval(() => { | |
| const dateBar = document.getElementById('dateBar'); | |
| if (dateBar) { | |
| dateBar.textContent = formatCurrentDate(); | |
| } | |
| let lastDateText = null; | |
| function startDateTimer() { | |
| if (dateTimerId) clearInterval(dateTimerId); | |
| // Initialize date bar immediately | |
| const initialDateBar = document.getElementById('dateBar'); | |
| if (initialDateBar) { | |
| const currentDateText = formatCurrentDate(); | |
| initialDateBar.textContent = currentDateText; | |
| lastDateText = currentDateText; | |
| } else { | |
| lastDateText = null; | |
| } | |
| // Check every minute and update only if the date has actually changed | |
| dateTimerId = setInterval(() => { | |
| const dateBar = document.getElementById('dateBar'); | |
| if (!dateBar) { | |
| lastDateText = null; | |
| return; | |
| } | |
| const newDateText = formatCurrentDate(); | |
| if (newDateText !== lastDateText) { | |
| dateBar.textContent = newDateText; | |
| lastDateText = newDateText; | |
| } |
| container.innerHTML = ''; | ||
| container.appendChild(dateBar); | ||
| container.insertAdjacentHTML('beforeend', ` | ||
| <div class="empty-state" style="width:100%"> |
There was a problem hiding this comment.
An inline style is used here to set width to 100%, which goes against the codebase convention of using CSS classes for styling. Consider adding this width rule to the .empty-state class in dashboard.css instead of using an inline style.
| <div class="empty-state" style="width:100%"> | |
| <div class="empty-state"> |
| // Start a timer to keep the date current | ||
| startDateTimer(); |
There was a problem hiding this comment.
The timer is started every time renderIframes() is called, but renderIframes() can be called multiple times (e.g., from saveServicesAndRefresh at line 325). While startDateTimer() does clear any existing timer before creating a new one, this pattern could lead to multiple unnecessary timer restarts. Consider checking if a timer is already running and only start it once during initialization, or ensure the timer continues running across re-renders without restarting.
v0 Session