-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[WEB-2341] feat: Add display filters and display properties to create/update view dialog #5451
Conversation
WalkthroughThe changes introduce enhancements to the Changes
Poem
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 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: 2
Outside diff range, codebase verification and nitpick comments (1)
web/core/components/workspace/views/form.tsx (1)
Line range hint
158-222
: Ensure thorough testing of the new display filter functionality.Given the complexity and importance of the new display filter functionality, it's crucial to ensure that all interactions and edge cases are thoroughly tested to prevent issues in production.
Would you like assistance in setting up comprehensive tests for this functionality?
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- web/core/components/views/form.tsx (3 hunks)
- web/core/components/workspace/views/form.tsx (3 hunks)
Additional comments not posted (4)
web/core/components/workspace/views/form.tsx (2)
7-7
: Approved import changes.The new types
IIssueDisplayFilterOptions
andIIssueDisplayProperties
are essential for the added functionality regarding display filters and properties.
11-11
: Approved component import changes.The import of the
DisplayFiltersSelection
component aligns with the new functionality for managing display filters.web/core/components/views/form.tsx (2)
8-8
: Approved import changes.The new types
IIssueDisplayProperties
andIIssueDisplayFilterOptions
are essential for the added functionality regarding display filters and properties.
37-37
: Approved update to defaultValues.The addition of the
group_by
property todisplay_filters
indefaultValues
enhances the structure for managing filter states. Ensure this new property integrates smoothly with existing data structures and workflows.Verify the integration of the
group_by
property with existing data structures:
{/* display filters dropdown */} | ||
<Controller | ||
control={control} | ||
name="display_filters" | ||
render={({ field: { onChange: onDisplayFiltersChange, value: displayFilters } }) => ( | ||
<Controller | ||
control={control} | ||
name="display_properties" | ||
render={({ field: { onChange: onDisplayPropertiesChange, value: displayProperties } }) => ( | ||
<FiltersDropdown title="Display"> | ||
<DisplayFiltersSelection | ||
layoutDisplayFiltersOptions={ISSUE_DISPLAY_FILTERS_BY_LAYOUT.my_issues.spreadsheet} | ||
displayFilters={displayFilters ?? {}} | ||
handleDisplayFiltersUpdate={(updatedDisplayFilter: Partial<IIssueDisplayFilterOptions>) => { | ||
onDisplayFiltersChange({ | ||
...displayFilters, | ||
...updatedDisplayFilter, | ||
}); | ||
}} | ||
displayProperties={displayProperties ?? {}} | ||
handleDisplayPropertiesUpdate={(updatedDisplayProperties: Partial<IIssueDisplayProperties>) => { | ||
onDisplayPropertiesChange({ | ||
...displayProperties, | ||
...updatedDisplayProperties, | ||
}); | ||
}} | ||
/> | ||
</FiltersDropdown> | ||
)} | ||
/> | ||
)} | ||
/> |
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.
Consider simplifying the nested controllers.
The nested Controller
components for display_filters
and display_properties
introduce complexity. While necessary for the functionality, consider simplifying the structure if possible to enhance maintainability.
name="display_filters" | ||
render={({ field: { onChange: onDisplayFiltersChange, value: displayFilters } }) => ( | ||
<> | ||
{/* layout dropdown */} | ||
<LayoutDropDown | ||
onChange={(selectedValue: EIssueLayoutTypes) => | ||
onDisplayFiltersChange({ | ||
...displayFilters, | ||
layout: selectedValue, | ||
}) | ||
} | ||
value={displayFilters.layout} | ||
/> | ||
|
||
{/* filters dropdown */} | ||
<Controller | ||
control={control} | ||
name="filters" | ||
render={({ field: { onChange, value: filters } }) => ( | ||
<FiltersDropdown title="Filters" tabIndex={3}> | ||
<FilterSelection | ||
filters={filters ?? {}} | ||
handleFiltersUpdate={(key, value) => { | ||
const newValues = filters?.[key] ?? []; | ||
|
||
if (Array.isArray(value)) { | ||
value.forEach((val) => { | ||
if (!newValues.includes(val)) newValues.push(val); | ||
}); | ||
} else { | ||
if (filters?.[key]?.includes(value)) newValues.splice(newValues.indexOf(value), 1); | ||
else newValues.push(value); | ||
} | ||
|
||
if (Array.isArray(value)) { | ||
value.forEach((val) => { | ||
if (!newValues.includes(val)) newValues.push(val); | ||
}); | ||
} else { | ||
if (filters?.[key]?.includes(value)) newValues.splice(newValues.indexOf(value), 1); | ||
else newValues.push(value); | ||
} | ||
onChange({ | ||
...filters, | ||
[key]: newValues, | ||
}); | ||
}} | ||
layoutDisplayFiltersOptions={ISSUE_DISPLAY_FILTERS_BY_LAYOUT.issues[displayFilters.layout]} | ||
labels={projectLabels ?? undefined} | ||
memberIds={projectMemberIds ?? undefined} | ||
states={projectStates} | ||
cycleViewDisabled={!currentProjectDetails?.cycle_view} | ||
moduleViewDisabled={!currentProjectDetails?.module_view} | ||
/> | ||
</FiltersDropdown> | ||
)} | ||
/> | ||
|
||
onChange({ | ||
...filters, | ||
[key]: newValues, | ||
}); | ||
}} | ||
layoutDisplayFiltersOptions={ISSUE_DISPLAY_FILTERS_BY_LAYOUT.issues.list} | ||
labels={projectLabels ?? undefined} | ||
memberIds={projectMemberIds ?? undefined} | ||
states={projectStates} | ||
cycleViewDisabled={!currentProjectDetails?.cycle_view} | ||
moduleViewDisabled={!currentProjectDetails?.module_view} | ||
{/* display filters dropdown */} | ||
<Controller | ||
control={control} | ||
name="display_properties" | ||
render={({ field: { onChange: onDisplayPropertiesChange, value: displayProperties } }) => ( | ||
<FiltersDropdown title="Display"> | ||
<DisplayFiltersSelection | ||
layoutDisplayFiltersOptions={ISSUE_DISPLAY_FILTERS_BY_LAYOUT.issues[displayFilters.layout]} | ||
displayFilters={displayFilters ?? {}} | ||
handleDisplayFiltersUpdate={(updatedDisplayFilter: Partial<IIssueDisplayFilterOptions>) => { | ||
onDisplayFiltersChange({ | ||
...displayFilters, | ||
...updatedDisplayFilter, | ||
}); | ||
}} | ||
displayProperties={displayProperties ?? {}} | ||
handleDisplayPropertiesUpdate={( | ||
updatedDisplayProperties: Partial<IIssueDisplayProperties> | ||
) => { | ||
onDisplayPropertiesChange({ | ||
...displayProperties, | ||
...updatedDisplayProperties, | ||
}); | ||
}} | ||
cycleViewDisabled={!currentProjectDetails?.cycle_view} | ||
moduleViewDisabled={!currentProjectDetails?.module_view} | ||
/> | ||
</FiltersDropdown> | ||
)} |
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.
Consider simplifying the nested controllers.
The nested Controller
components for display_filters
and display_properties
introduce complexity. While necessary for the functionality, consider simplifying the structure if possible to enhance maintainability.
This PR add the display filters and properties to create/update view dialog
Screen.Recording.2024-08-28.at.4.36.45.PM.mov
Summary by CodeRabbit
New Features
ProjectViewForm
andWorkspaceViewForm
components with improved display filter management.DisplayFiltersSelection
component for better handling of filter updates.Bug Fixes