fix(datePicker): prevent year, month change in showMonthAndYearPickers mode #3088
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@KumJungMin is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes primarily address issues related to the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant DatePickerComponent
participant CalendarComponent
participant useCalendarPicker
User->>DatePickerComponent: Open Date Picker
DatePickerComponent->>useCalendarPicker: Initialize
useCalendarPicker->>useCalendarPicker: Check if header is expanded
alt Header is not expanded
useCalendarPicker-->>DatePickerComponent: Skip scrolling
else Header is expanded
useCalendarPicker-->>DatePickerComponent: Scroll to selected month/year
end
DatePickerComponent-->>User: Display Date Picker
User->>DatePickerComponent: Toggle Picker
DatePickerComponent->>CalendarComponent: Toggle Month/Year Picker
CalendarComponent-->>DatePickerComponent: Update view
DatePickerComponent-->>User: Display updated picker
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 1
Outside diff range and nitpick comments (2)
packages/components/calendar/src/use-calendar-picker.ts (2)
Line range hint
86-86: Correct dependencies in useCallback hook.The
useCallbackhook at line 86 specifiesisHeaderExpandedas a dependency, which is not used within the callback. Additionally, it does not includegetItemsRefMap. Update the dependencies to ensure correct behavior:- }, [state, isHeaderExpanded]); + }, [state, getItemsRefMap]);
Line range hint
184-184: Update dependencies for keyboard navigation handling.The
useCallbackhook at line 184 is missing several dependencies which are crucial for its operation. It also includesstatewhich is not directly used. Update the dependencies to ensure that all necessary values are tracked:- }, [state]); + }, [getItemsRefMap, months.length, headerRef, setIsHeaderExpanded]);
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/components/calendar/src/use-calendar-picker.ts (1 hunks)
Additional Context Used
Biome (13)
packages/components/calendar/src/use-calendar-picker.ts (13)
4-5: All these imports are only used as types.
86-86: This hook does not specify all of its dependencies: getItemsRefMap
86-86: This hook specifies more dependencies than necessary: isHeaderExpanded
109-109: This let declares a variable that is only assigned once.
117-117: This hook does not specify all of its dependencies: date.month
117-117: This hook does not specify all of its dependencies: date.year
172-172: This hook does not specify all of its dependencies: scrollTo
172-172: This hook specifies more dependencies than necessary: state
184-184: This hook does not specify all of its dependencies: getItemsRefMap
184-184: This hook does not specify all of its dependencies: months.length
184-184: This hook specifies more dependencies than necessary: state
184-184: This hook does not specify all of its dependencies: headerRef?.current?.focus
184-184: This hook does not specify all of its dependencies: setIsHeaderExpanded
Additional comments not posted (1)
packages/components/calendar/src/use-calendar-picker.ts (1)
118-119: Ensure proper handling of component state when header is not expanded.The addition of the conditional check
if (!isHeaderExpanded) return;effectively prevents unnecessary operations when the header is not expanded, aligning with the PR's objective to maintain correct month and year information.
KumJungMin
changed the title
KumJungMin
changed the title
KumJungMin
changed the title
KumJungMin
changed the title
KumJungMin
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .changeset/tidy-squids-knock.md (1 hunks)
Additional comments not posted (1)
.changeset/tidy-squids-knock.md (1)
1-5: The changeset file is well-formed and clearly documents the patch.
KumJungMin
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .changeset/tidy-squids-knock.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .changeset/tidy-squids-knock.md
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
packages/components/date-picker/__tests__/date-picker.test.tsx (1)
461-461: Improve the test case description for clarity.Consider enhancing the description of the test case to explicitly state what is being tested and expected, such as "Ensure the selected date remains unchanged when toggling the date picker in showMonthAndYearPickers mode."
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/components/date-picker/tests/date-picker.test.tsx (1 hunks)
Additional Context Used
Biome (20)
packages/components/date-picker/__tests__/date-picker.test.tsx (20)
27-27: Unexpected any. Specify a different type.
41-41: This variable implicitly has the any type.
67-100: This function expression can be turned into an arrow function.
102-163: This function expression can be turned into an arrow function.
164-169: This function expression can be turned into an arrow function.
170-175: This function expression can be turned into an arrow function.
192-234: This function expression can be turned into an arrow function.
235-268: This function expression can be turned into an arrow function.
269-294: This function expression can be turned into an arrow function.
295-343: This function expression can be turned into an arrow function.
344-370: This function expression can be turned into an arrow function.
373-424: This function expression can be turned into an arrow function.
425-460: This function expression can be turned into an arrow function.
461-484: This function expression can be turned into an arrow function.
372-485: This function expression can be turned into an arrow function.
7-8: Some named imports are only used as types.
68-68: This let declares a variable that is only assigned once.
70-70: This let declares a variable that is only assigned once.
76-76: This let declares a variable that is only assigned once.
103-103: This let declares a variable that is only assigned once.
Additional comments not posted (1)
packages/components/date-picker/__tests__/date-picker.test.tsx (1)
462-482: The implementation of the test case looks solid.The test case effectively simulates the user interaction and correctly checks the expected outcomes. Good use of
triggerPressand visibility checks.
|
i added the changelog and test code :)
|
|
Thank you! @KumJungMin |
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
packages/components/date-picker/__tests__/date-picker.test.tsx (3)
Line range hint
67-100: Convert traditional function expressions to arrow functions for consistency and conciseness.Consider refactoring the traditional function expressions into arrow functions. This will not only make the code more consistent with modern JavaScript practices but also enhance readability. Here's an example of how you can refactor one of the functions:
- it("should render a datepicker with a specified date", function () { + it("should render a datepicker with a specified date", () => {Repeat this pattern for other function expressions throughout the file.
Also applies to: 102-163, 164-169, 170-175, 192-234, 235-268, 269-294, 295-343, 344-370, 373-424, 425-460, 461-482, 372-483
Line range hint
27-27: Improve type safety by specifying explicit types instead ofany.The use of
anytype reduces type safety and can lead to maintenance issues. Consider specifying more explicit types for better type checking and code clarity. For instance:- function getTextValue(el: any) { + function getTextValue(el: HTMLElement) {Also applies to: 41-41
Line range hint
68-68: Useconstfor variables that are not reassigned.Variables that are not reassigned after their initial assignment should be declared with
constinstead ofletfor better code clarity and to enforce immutability where applicable. For example:- let combobox = getAllByRole("group")[0]; + const combobox = getAllByRole("group")[0];Apply this change to similar cases in the file.
Also applies to: 70-70, 76-76, 103-103
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/components/date-picker/tests/date-picker.test.tsx (1 hunks)
Additional Context Used
Biome (20)
packages/components/date-picker/__tests__/date-picker.test.tsx (20)
27-27: Unexpected any. Specify a different type.
41-41: This variable implicitly has the any type.
67-100: This function expression can be turned into an arrow function.
102-163: This function expression can be turned into an arrow function.
164-169: This function expression can be turned into an arrow function.
170-175: This function expression can be turned into an arrow function.
192-234: This function expression can be turned into an arrow function.
235-268: This function expression can be turned into an arrow function.
269-294: This function expression can be turned into an arrow function.
295-343: This function expression can be turned into an arrow function.
344-370: This function expression can be turned into an arrow function.
373-424: This function expression can be turned into an arrow function.
425-460: This function expression can be turned into an arrow function.
461-482: This function expression can be turned into an arrow function.
372-483: This function expression can be turned into an arrow function.
7-8: Some named imports are only used as types.
68-68: This let declares a variable that is only assigned once.
70-70: This let declares a variable that is only assigned once.
76-76: This let declares a variable that is only assigned once.
103-103: This let declares a variable that is only assigned once.
Additional comments not posted (1)
packages/components/date-picker/__tests__/date-picker.test.tsx (1)
461-482: Ensure the new test case correctly verifies the date remains unchanged inshowMonthAndYearPickersmode.This test case effectively checks that the selected date is maintained when the picker is opened in
showMonthAndYearPickersmode, aligning with the PR's objective to fix the issue where the month and year were being reset.
wingkwong
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .changeset/tidy-squids-knock.md (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- .changeset/tidy-squids-knock.md
📝 Description
Summary by CodeRabbit
Bug Fixes
showMonthAndYearPickersmode for@nextui-org/calendarand@nextui-org/date-pickerpackages.showMonthAndYearPickersmode.Improvements