Completed the datepicker feature in user standup updates and fixed the hover issue on dates #851
Conversation
…issue on standup dates
After making changes there was an update on develop branch so as stated in contributing.md I pulled those changes and rebased it as we were told to do in the readme. |
WalkthroughThe changes introduce enhancements for date handling and scrolling functionality. In the test file, new functions for counting Sundays and calculating scroll position are added alongside test cases to validate correct scrolling behavior based on a selected date. In the main script, the table header is updated with a dynamic date picker and a new scroll-to-date function is implemented. Additionally, the hover effect in styles is refined by shifting from row-based to cell-based interactions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DatePicker
participant ScrollFunction
participant CountSundays
participant Table
User->>DatePicker: Select date
DatePicker->>ScrollFunction: Trigger change event with date
ScrollFunction->>CountSundays: Calculate Sundays between dates
CountSundays-->>ScrollFunction: Return count
ScrollFunction->>Table: Compute and apply scroll position
Poem
✨ Finishing Touches
🪧 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: 0
🧹 Nitpick comments (3)
standup/script.js (1)
385-420: Consider adding error handling for invalid dates.The
scrollToSelectedDatefunction should handle potential edge cases:
- Invalid date strings
- Null or undefined values
function scrollToSelectedDate(date) { + if (!date) return; const selectedDate = new Date(date); + if (isNaN(selectedDate.getTime())) return; selectedDate.setHours(0, 0, 0, 0); if (selectedDate < startDate || selectedDate > endDate) { return; }__tests__/standup/standup.test.js (2)
191-224: Consider reducing test flakiness.The test relies on fixed timeouts which could lead to flaky behavior. Consider using more reliable waiting mechanisms.
- await page.waitForTimeout(1000); + await page.waitForFunction(() => { + const container = document.getElementById('table-container'); + return container.scrollLeft > 0; + });
226-259: Consider adding more edge cases.The test could be expanded to cover additional scenarios:
- Invalid date strings
- Null or undefined values
- Dates before the start date
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
__tests__/standup/standup.test.js(2 hunks)standup/script.js(2 hunks)standup/style.css(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
standup/style.css (1)
158-160: LGTM! The hover effect is now correctly scoped to individual cells.The changes successfully address the hover issue mentioned in the PR objectives by shifting from row-based to cell-based hover interactions.
standup/script.js (2)
124-143: LGTM! The datepicker implementation follows best practices.The changes align with past review feedback by:
- Creating separate DOM elements instead of using string HTML.
- Using proper element hierarchy and styling.
146-149: LGTM! Event handling is properly implemented.The event listener is correctly set up to handle date selection and trigger scrolling.
__tests__/standup/standup.test.js (1)
25-52: LGTM! The utility functions are well-implemented.The functions correctly handle:
- Sunday counting logic
- Scroll position calculation
Issue on standup dates
Date: 24-09-2024
Developer Name: Ankit Kanyal
Issue Ticket Number
#830
Description
So I have added a datepicker in user standup update section when clicked the user is automatically scrolled to that particular standup date.Also I have fixed hover issues when user hover over standup dates .Previously when user hover over any date hover effect is applied to entire row.
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
standup-demo.mp4
Test Coverage
Screenshot 1
Group tests are passing.
Additional Notes
Summary by CodeRabbit
New Features
Style