-
Notifications
You must be signed in to change notification settings - Fork 61
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
refactor(my-pages): replace filter with UI core filter #17510
base: main
Are you sure you want to change the base?
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. WalkthroughThe pull request involves a comprehensive refactoring of filter components across multiple files in the my-pages portals. The primary changes include removing the standalone Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
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
Documentation and Community
|
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 (8)
libs/portals/my-pages/finance/src/components/HousingBenefitsPayments/HousingBenefitsFilter.tsx (1)
209-226
: Consider adding loading and error states for export actions.The export functionality should handle loading and error states to provide better user feedback during file generation.
<DropdownExport + isLoading={isExporting} + onError={(error) => { + // Handle export error + console.error('Export failed:', error); + }} onGetCSV={() => exportHousingBenefitFiles(payments?.data ?? [], 'csv')} onGetExcel={() => exportHousingBenefitFiles(payments?.data ?? [], 'xlsx') } />libs/portals/my-pages/documents/src/components/DocumentFilter/DocumentsFilterV2.tsx (1)
101-101
: Consider using consistent input sizes across filters.The input size is set to
xs
, which differs from other filter implementations. Consider standardizing input sizes across all filter components for consistency.libs/portals/my-pages/assets/src/screens/WorkMachinesOverview/WorkMachinesOverview.tsx (1)
224-224
: Remove empty grid column.The grid column with an empty fragment (
{}
) appears to serve no purpose and should be removed.-<GridColumn span={'4/12'}>{}</GridColumn>
libs/portals/my-pages/finance/src/screens/FinanceTransactions/FinanceTransactions.tsx (2)
126-165
: Consider optimizing date filter initialization.The date filter initialization could be simplified by setting initial dates in a single useEffect, reducing potential re-renders.
- useEffect(() => { - setFromDate(backInTheDay) - setToDate(new Date()) - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []) + useEffect(() => { + const today = new Date() + setFromDate(sub(today, { months: 3 })) + setToDate(today) + }, [])
146-166
: Consider memoizing FilterMultiChoice categories.The categories array is recreated on every render. Consider memoizing it using useMemo to optimize performance.
+ const filterCategories = useMemo(() => [ + { + id: 'flokkur', + label: formatMessage(messages.transactionsLabel), + labelAs: 'h2', + selected: dropdownSelect ? [...dropdownSelect] : [], + filters: chargeTypeSelect, + inline: false, + singleOption: false, + }, + ], [dropdownSelect, chargeTypeSelect, formatMessage]) <FilterMultiChoice labelClear={formatMessage(m.clearSelected)} singleExpand={true} onChange={({ selected }) => setDropdownSelect(selected)} onClear={() => setEmptyChargeTypes()} - categories={[...]} + categories={filterCategories} />libs/portals/my-pages/finance/src/components/DocumentScreen/DocumentScreen.tsx (2)
113-120
: Simplify flex container properties.The Box component's flex properties can be simplified by using the Stack component from island-ui/core.
- <Box - display="flex" - flexDirection="row" - justifyContent="flexStart" - flexWrap="wrap" - rowGap={2} - columnGap={2} - > + <Stack space={2} direction="row" align="center" wrap>
Line range hint
167-192
: Enhance date picker accessibility.Consider adding aria-labels and helper text to improve the date picker's accessibility.
<DatePicker label={formatMessage(m.datepickerFromLabel)} placeholderText={formatMessage(m.datepickLabel)} locale="is" backgroundColor="blue" size="xs" handleChange={(d) => setFromDate(d)} selected={fromDate} appearInline + aria-label={formatMessage(m.datepickerFromLabel)} + helperText={formatMessage(m.datepickerHelperText)} />libs/portals/my-pages/finance/src/components/FinanceTransactionPeriods/FinanceTransactionPeriodsFilter.tsx (1)
171-202
: Enhance type safety for filter categories.Consider creating a dedicated type for filter categories to improve type safety and maintainability.
+ type FilterCategory = { + id: 'years' | 'flokkur'; + label: string; + selected: string[]; + filters: Array<{ label: string; value: string }>; + inline: boolean; + singleOption: boolean; + }; <FilterMultiChoice labelClear={formatMessage(m.clearSelected)} singleExpand={true} - categories={[...]} + categories={categories as FilterCategory[]} />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
libs/portals/my-pages/assets/src/screens/WorkMachinesOverview/WorkMachinesOverview.tsx
(3 hunks)libs/portals/my-pages/core/src/components/Filter/Filter.css.ts
(0 hunks)libs/portals/my-pages/core/src/components/Filter/Filter.tsx
(0 hunks)libs/portals/my-pages/core/src/index.ts
(1 hunks)libs/portals/my-pages/documents/src/components/DocumentFilter/DocumentsFilterV2.tsx
(2 hunks)libs/portals/my-pages/finance/src/components/DocumentScreen/DocumentScreen.tsx
(4 hunks)libs/portals/my-pages/finance/src/components/FinanceTransactionPeriods/FinanceTransactionPeriodsFilter.tsx
(2 hunks)libs/portals/my-pages/finance/src/components/HousingBenefitsPayments/HousingBenefitsFilter.tsx
(2 hunks)libs/portals/my-pages/finance/src/screens/FinanceLoans/FinanceLoans.tsx
(1 hunks)libs/portals/my-pages/finance/src/screens/FinanceTransactions/FinanceTransactions.tsx
(2 hunks)
💤 Files with no reviewable changes (2)
- libs/portals/my-pages/core/src/components/Filter/Filter.css.ts
- libs/portals/my-pages/core/src/components/Filter/Filter.tsx
✅ Files skipped from review due to trivial changes (1)
- libs/portals/my-pages/finance/src/screens/FinanceLoans/FinanceLoans.tsx
🧰 Additional context used
📓 Path-based instructions (7)
libs/portals/my-pages/assets/src/screens/WorkMachinesOverview/WorkMachinesOverview.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/portals/my-pages/finance/src/components/FinanceTransactionPeriods/FinanceTransactionPeriodsFilter.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/portals/my-pages/finance/src/components/HousingBenefitsPayments/HousingBenefitsFilter.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/portals/my-pages/finance/src/screens/FinanceTransactions/FinanceTransactions.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/portals/my-pages/documents/src/components/DocumentFilter/DocumentsFilterV2.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/portals/my-pages/finance/src/components/DocumentScreen/DocumentScreen.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/portals/my-pages/core/src/index.ts (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (8)
libs/portals/my-pages/core/src/index.ts (1)
1-74
: LGTM! Well-organized exports.The exports are properly structured and grouped by type (components, hooks, utils). The removal of the
Filter
component aligns with the PR objective of replacing it with a UI core filter.libs/portals/my-pages/finance/src/components/HousingBenefitsPayments/HousingBenefitsFilter.tsx (1)
81-207
: LGTM! Well-structured filter implementation.The filter implementation is clean and well-organized:
- Uses the new UI core
Filter
component- Properly handles payment type selection
- Includes date filtering with month/year selection
- Maintains clear separation between filter controls and export functionality
libs/portals/my-pages/documents/src/components/DocumentFilter/DocumentsFilterV2.tsx (1)
1-27
: LGTM! Well-organized imports and type definitions.The imports are properly organized and the type definitions are clear and well-structured.
libs/portals/my-pages/assets/src/screens/WorkMachinesOverview/WorkMachinesOverview.tsx (1)
206-219
: LGTM! Clean implementation of export functionality.The export functionality is well-implemented using the
DropdownMenu
component with clear actions for CSV and Excel exports.libs/portals/my-pages/finance/src/screens/FinanceTransactions/FinanceTransactions.tsx (2)
1-40
: LGTM! Clean import organization.The imports are well-organized, separating external dependencies from internal modules.
222-239
: LGTM! Well-structured export functionality.The export and print buttons are properly integrated into the Inline component, maintaining consistent spacing and alignment.
libs/portals/my-pages/finance/src/components/DocumentScreen/DocumentScreen.tsx (1)
208-215
: LGTM! Well-implemented export functionality.The export options are properly implemented with clear separation of concerns using dedicated utility functions.
libs/portals/my-pages/finance/src/components/FinanceTransactionPeriods/FinanceTransactionPeriodsFilter.tsx (1)
206-217
: LGTM! Well-structured print functionality.The print button is properly integrated into the Inline component with consistent styling.
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.
LGTM
My pages - Filter
Delete special my pages filter and use Island UI component instead
Checklist:
Summary by CodeRabbit
Release Notes
Refactoring
Filter
component and associated CSS stylesInline
componentComponent Changes
Performance
These changes primarily focus on improving the structure and maintainability of the UI components without significantly altering core functionality.