-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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: module date picker #6691
fix: module date picker #6691
Conversation
WalkthroughThe changes consolidate date selection by replacing the old Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DRD as DateRangeDropdown
participant C as Parent Component
U->>DRD: Select date range (from, to)
DRD->>C: Trigger onSelect({from, to})
C->>C: Process selected date range (format, update via handler, submit changes)
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (2)
web/core/components/modules/module-card-item.tsx (1)
240-260
: Consider using translations for placeholder texts.While the implementation of DateRangeDropdown is correct, the placeholder texts are hardcoded strings instead of using the translation function like in other components.
Consider using the translation function for consistency with other components:
placeholder={{ - from: "Start date", - to: "End date", + from: t("start_date"), + to: t("end_date"), }}web/core/components/modules/analytics-sidebar/root.tsx (1)
420-456
: Nested Controllers implementation is functional but complex.The implementation uses nested React Hook Form Controller components to handle both date fields, which works but introduces some complexity.
Consider simplifying the nested Controllers with a single Controller that manages both date fields:
<div className="h-7 w-3/5"> <Controller control={control} - name="start_date" - render={({ field: { value: startDateValue, onChange: onChangeStartDate } }) => ( - <Controller - control={control} - name="target_date" - render={({ field: { value: endDateValue, onChange: onChangeEndDate } }) => { + name={["start_date", "target_date"]} + render={({ field }) => { const startDate = getDate(startDateValue); const endDate = getDate(endDateValue); return ( <DateRangeDropdown buttonContainerClassName="w-full" buttonVariant="background-with-text" value={{ from: startDate, to: endDate, }} onSelect={(val) => { - onChangeStartDate(val?.from ? renderFormattedPayloadDate(val.from) : null); - onChangeEndDate(val?.to ? renderFormattedPayloadDate(val.to) : null); + // Update form values for both fields + control.setValue("start_date", val?.from ? renderFormattedPayloadDate(val.from) : null); + control.setValue("target_date", val?.to ? renderFormattedPayloadDate(val.to) : null); handleDateChange(val?.from, val?.to); }} placeholder={{ from: t("start_date"), to: t("target_date"), }} disabled={!isEditingAllowed || isArchived} /> ); - }} - /> )} /> </div>This is just a suggestion - the current implementation is functional and works correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/core/components/dropdowns/date-range.tsx
(0 hunks)web/core/components/modules/analytics-sidebar/root.tsx
(4 hunks)web/core/components/modules/module-card-item.tsx
(4 hunks)web/core/components/modules/module-list-item-action.tsx
(3 hunks)
💤 Files with no reviewable changes (1)
- web/core/components/dropdowns/date-range.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (10)
web/core/components/modules/module-list-item-action.tsx (4)
7-7
: Unused imports have been removed.The import list has been cleaned up by removing unused icon imports, leaving only
SquareUser
which is actually used in the component.
21-21
: DateDropdown replaced with DateRangeDropdown.This change aligns with the PR objective of reverting the module date picker back to a date range picker.
56-56
: LGTM!This variable sets up a conditional check to determine whether icons should be displayed in the date dropdown based on the presence of dates.
141-161
: DateRangeDropdown implementation properly handles both start and end dates.The implementation correctly:
- Passes both start and end dates as a single value object
- Properly formats dates for the API payload in the onSelect handler
- Sets appropriate placeholders with translations
- Controls icon visibility based on whether dates exist
The change consolidates the UI and provides a better user experience by allowing date range selection in a single component.
web/core/components/modules/module-card-item.tsx (3)
7-7
: Unused icon imports have been removed.The import list has been simplified by removing calendar-related icons that were likely used with the previous individual date pickers.
29-29
: DateDropdown replaced with DateRangeDropdown.This change aligns with the PR objective of reverting to a date range picker component.
69-69
: LGTM!This variable determines whether icons should be displayed in the date dropdown based on the presence of dates.
web/core/components/modules/analytics-sidebar/root.tsx (3)
45-45
: DateDropdown replaced with DateRangeDropdown.This import change aligns with the PR objective of using a date range picker component.
197-207
: Updated date handling function to support date ranges.The function has been properly updated to:
- Accept separate start and target date parameters
- Format both dates for the API payload
- Submit both dates in a single update
This implementation correctly supports the new date range picker functionality.
418-418
: Updated label text to reflect the date range functionality.The label now uses "date_range" rather than individual labels for start and target dates, which aligns with the UI changes.
* fix: issue activity for project id validation (#6668) * fix: work item attachment count mutation (#6670) * updated the action to modify the release build assets (#6669) * feat: russian translation (#6666) * chore: ru translation updated (#6672) * fix: state drop down refactor * fix: intake work item creation refactor * fix: cleanup for deprecated functions * fix: date range picker on cycles and modules list (#6676) * fix: Handled workspace switcher closing on click * fix: replaced date range picker with date picker at some places * chore: add common translation keys (#6688) * chore: add missing translation keys * chore: add russian translation keys * fix: issue activity task (#6689) * changed github workflow action ubuntu version to `ubuntu-22.04` (#6683) * chore: update russian translation (#6682) * chore: update russian translation * chore: rename issues to work items in russian translation * [PE-275] chore: editor line spacing variables (#6678) * chore: variable editor line spacing * chore: variable list spacing --------- Co-authored-by: Aaryan Khandelwal <aaryankhandu123@gmail.com> * [WEB-3475] fix: cycle dates dropdown (#6690) * fix: Handled workspace switcher closing on click * fix: Cycle date picker * fix: Made onSelect optional in range range component * fix: module date picker (#6691) * fix: Handled workspace switcher closing on click * fix: reverted module date picker changes * chore: extended sidebar improvement (#6693) * feat: italian translations (#6692) * Create translations.json - ITALIAN translation (#6667) * chore: italian translation updated * feat: italian translation added * fix: module end date translation --------- Co-authored-by: Nicolas Bossi <nicolasbossi@gmail.com> Co-authored-by: gakshita <akshitagoyal1516@gmail.com> * fix: attachment item created by (#6695) * fix: module flicker issue on property updation (#6699) * [WEB-3477] fix: mutation issue on moving work items for a manually ended cycle (#6696) * fix: package version update * fix: esbuild version fix * fix: package license repliation * [WEB-3488] improvement: assignee validation for work item creation (#6701) * fix: work item assignee update validation (#6704) --------- Co-authored-by: Nikhil <118773738+pablohashescobar@users.noreply.github.com> Co-authored-by: Anmol Singh Bhatia <121005188+anmolsinghbhatia@users.noreply.github.com> Co-authored-by: Manish Gupta <59428681+mguptahub@users.noreply.github.com> Co-authored-by: Nikita Mitasov <32384814+ch4og@users.noreply.github.com> Co-authored-by: Akshita Goyal <36129505+gakshita@users.noreply.github.com> Co-authored-by: Aaryan Khandelwal <65252264+aaryan610@users.noreply.github.com> Co-authored-by: Akshat Jain <akshatjain9782@gmail.com> Co-authored-by: Lakhan Baheti <94619783+1akhanBaheti@users.noreply.github.com> Co-authored-by: Aaryan Khandelwal <aaryankhandu123@gmail.com> Co-authored-by: Nicolas Bossi <nicolasbossi@gmail.com> Co-authored-by: gakshita <akshitagoyal1516@gmail.com> Co-authored-by: Prateek Shourya <prateekshourya29@gmail.com>
Description
This PR reverts the module date picker back to date range picker
Summary by CodeRabbit