-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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.
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
fix: datepicker doesnt close on first selection of date on month change #34679
base: release
Are you sure you want to change the base?
fix: datepicker doesnt close on first selection of date on month change #34679
Conversation
WalkthroughThe recent changes introduced tests for the DatePicker widget's property pane with JavaScript bindings and refactored the date change handling in the DatePicker component. Additionally, utility functions for testing were integrated to enhance the test suite, focusing on improved navigation and date selection functionalities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TestSuite as Test Suite
participant Datepicker as DatePickerComponent
User ->> TestSuite: Run test
TestSuite ->> Datepicker: Navigate to DatePicker widget
TestSuite ->> Datepicker: Select random date
Note right of Datepicker: Changes month
TestSuite ->> Datepicker: Click to close
Datepicker ->> TestSuite: DatePicker closed
Poem
Tip AI model upgrade
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker4_spec.js (1)
33-40
: Use a more descriptive test name.The test name should clearly convey the intent of the test.
- it("1.should close datepicker on single click when month is changed", function () { + it("should close the datepicker on a single click after changing the month", function () {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker4_spec.js (1 hunks)
- app/client/src/widgets/DatePickerWidget2/component/index.tsx (2 hunks)
Additional context used
Path-based instructions (2)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker4_spec.js (1)
Pattern
**/*.*
: Do not use cy.wait in code.
Do call the add function.
Do not keep duplicate lines in code.
Descriptive test names are used to clearly convey the intent of each test.app/client/src/widgets/DatePickerWidget2/component/index.tsx (1)
Pattern
**/*.*
: Do not use cy.wait in code.
Do call the add function.
Do not keep duplicate lines in code.
Descriptive test names are used to clearly convey the intent of each test.
Additional comments not posted (4)
app/client/cypress/e2e/Regression/ClientSide/Widgets/Datepicker/DatePicker4_spec.js (1)
17-31
: Verify date selection logic.Ensure that the selected date is within the valid range of dates for the DatePicker widget.
app/client/src/widgets/DatePickerWidget2/component/index.tsx (3)
484-491
: Verify date formatting logic.Ensure that the date formatting logic is consistent with the rest of the application.
380-380
: Verify edge case handling inhandleDateChange
.Ensure that the
handleDateChange
function correctly handles all edge cases, such as null or invalid dates.
483-491
: Verify synchronization of state update and prop call.Ensure that the state update and prop call in the
handleDateChange
function are correctly synchronized to avoid potential issues.
import { | ||
draggableWidgets, | ||
entityExplorer, | ||
agHelper, | ||
} from "../../../../../support/Objects/ObjectsCore"; | ||
import * as _ from "../../../../../support/Objects/ObjectsCore"; | ||
import { datePickerlocators } from "../../../../../locators/WidgetLocators"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused import.
The import statement for _
from "../../../../../support/Objects/ObjectsCore"
is unused and can be removed to keep the code clean.
- import * as _ from "../../../../../support/Objects/ObjectsCore";
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { | |
draggableWidgets, | |
entityExplorer, | |
agHelper, | |
} from "../../../../../support/Objects/ObjectsCore"; | |
import * as _ from "../../../../../support/Objects/ObjectsCore"; | |
import { datePickerlocators } from "../../../../../locators/WidgetLocators"; | |
import { | |
draggableWidgets, | |
entityExplorer, | |
agHelper, | |
} from "../../../../../support/Objects/ObjectsCore"; | |
import { datePickerlocators } from "../../../../../locators/WidgetLocators"; |
@ApekshaBhosale I accidentally deleted my forked repository, but I have raised PR with the suggested changes addressing the comments. Could you please review this PR? |
I hope you're well. I’m curious and excited to hear your thoughts on my pull request raised a while ago. Looking forward to your review! |
Fixes issue #30701
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
onChange
function in the DatePicker component to format and update the date before invoking the callback, enhancing the reliability of date formatting.