Skip to content

feat: adding checkboxes in lists#479

Merged
marcinkrasowski merged 16 commits intomainfrom
feature/adding-checkboxes-in-lists
Dec 18, 2025
Merged

feat: adding checkboxes in lists#479
marcinkrasowski merged 16 commits intomainfrom
feature/adding-checkboxes-in-lists

Conversation

@lukasz-hycom
Copy link
Copy Markdown
Contributor

@lukasz-hycom lukasz-hycom commented Dec 8, 2025

What does this PR do?

  • enabling selection of multiple rows in DataList tables and performing bulk actions on selected items

Key Changes

  • extending the DataList component with a new column with checkboxes
  • extending the DataView component with bulk actions bar
  • implementing bulk actions in the invoice list block

Summary by CodeRabbit

  • New Features

    • Row selection added to invoice, notification, order, product, and ticket lists with controlled selection state and reset on filters.
    • Bulk actions UI in list views with customizable pluralized labels and a locale-aware "download all" action.
    • DataView/DataList expose selection and bulk-action hooks; interactive stories added demonstrating selection and bulk actions.
  • Bug Fixes

    • Fixed incorrect price rendering in list/data view.

✏️ Tip: You can customize this high-level summary in your review settings.

@lukasz-hycom lukasz-hycom self-assigned this Dec 8, 2025
@vercel
Copy link
Copy Markdown

vercel Bot commented Dec 8, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
o2s-docs Skipped Skipped Dec 18, 2025 11:39am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 8, 2025

Walkthrough

Adds controlled row-selection and bulk-action support to DataView/DataList, propagates three invoice-list CMS fields (enableRowSelection, bulkActionsLabel, downloadAllButtonLabel) through models and mappers, and implements locale-aware invoice bulk-download and selection state in invoice-list frontend.

Changes

Cohort / File(s) Summary
Core UI: DataView & DataList
packages/ui/src/components/DataView/DataView.types.ts, packages/ui/src/components/DataView/DataView.tsx, packages/ui/src/components/DataList/DataList.types.ts, packages/ui/src/components/DataList/DataList.tsx, packages/ui/src/components/DataList/DataList.stories.tsx
Added controlled row-selection props (enableRowSelection, selectedRows, onSelectionChange) and bulk-action hooks (bulkActions, bulkActionsLabel) to DataView/DataList; DataList renders header and per-row checkboxes with selection handlers; stories added demonstrating selection and bulk actions.
Invoice list: model & API harmonization
packages/blocks/invoice-list/src/api-harmonization/invoice-list.model.ts, packages/blocks/invoice-list/src/api-harmonization/invoice-list.mapper.ts, packages/framework/src/modules/cms/models/blocks/invoice-list.model.ts
Added optional fields to InvoiceListBlock: enableRowSelection?: boolean, bulkActionsLabel?: string, downloadAllButtonLabel?: string; mapper propagates those fields from CMS. Adjusted Invoice.totalAmountDue and totalNetAmountDue types to the upstream shape.
Invoice list: frontend behavior
packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx, packages/blocks/invoice-list/src/frontend/InvoiceList.client.stories.tsx
Introduced selectedRows state, reset selection on data reset/filter, wired selection and bulk-action props to DataView, implemented locale-aware bulk-download flow with per-item error handling and small inter-download delay; story adjusted nested currency fields.
Invoice list: CMS integrations / mocks
packages/integrations/contentful-cms/.../cms.invoice-list.mapper.ts, packages/integrations/mocked/.../cms.invoice-list.mapper.ts, packages/integrations/strapi-cms/.../cms.invoice-list.mapper.ts
Contentful and mocked mappers populate enableRowSelection, bulkActionsLabel, downloadAllButtonLabel with localized values; Strapi mapper added placeholders/TODOs for these fields.
Other list blocks: types & client wiring
packages/blocks/notification-list/src/frontend/..., packages/blocks/order-list/src/frontend/..., packages/blocks/product-list/src/frontend/..., packages/blocks/ticket-list/src/frontend/..., packages/blocks/*/src/frontend/*.types.ts
Added enableRowSelection?: boolean to block props and introduced local selectedRows state in client components; selection resets on filter/reset and selection props/getRowKey passed to DataView/DataList.
UI stories & examples
packages/ui/src/components/DataList/DataList.stories.tsx, packages/ui/src/components/DataGrid/DataGrid.stories.tsx
Added stories demonstrating row selection and bulk actions; adjusted imports for types and components.
Renderer & import alignments
packages/ui/src/lib/renderCell.tsx, packages/ui/src/components/Cards/InformativeCard/InformativeCard.tsx, packages/ui/src/components/Cards/PricingCard/PricingCard.tsx, packages/ui/src/components/LinkList/LinkList.tsx, packages/ui/src/components/DataGrid/DataGrid.tsx
Fixed price renderer value access; migrated several relative imports to package-scoped imports for shared components/elements.
Changesets
.changeset/mean-shoes-behave.md, .changeset/shy-bobcats-count.md
Added changeset entries for affected packages.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User
  participant CMS
  participant InvoiceList as InvoiceList.client
  participant DataView
  participant DataList
  participant PDF as PDF endpoint

  Note over CMS,InvoiceList: CMS supplies enableRowSelection, bulkActionsLabel, downloadAllButtonLabel
  User->>InvoiceList: open invoice list
  InvoiceList->>DataView: pass data + enableRowSelection, selectedRows, onSelectionChange, bulkActionsLabel, bulkActions, getRowKey
  DataView->>DataList: forward selection props and current page data
  User->>DataList: click row checkbox / header checkbox
  DataList->>InvoiceList: onSelectionChange(selectedKeys)
  InvoiceList->>InvoiceList: update selectedRows
  alt User triggers bulk download
    loop for each selected invoice
      InvoiceList->>PDF: fetch invoice PDF (locale-aware)
      PDF-->>InvoiceList: PDF blob or error
      InvoiceList->>User: trigger download or surface per-item error
      Note right of InvoiceList: short delay between downloads
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas to review closely:

  • packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx — bulk-download request construction, locale handling, per-item error handling, download triggering and delay logic.
  • packages/ui/src/components/DataList/DataList.tsx & packages/ui/src/components/DataView/DataView.tsx — header checkbox checked/indeterminate logic, performance of current-page key computation, and integration of selection props.
  • Cross-package typing and consistency for getRowKey and selectedRows (string vs number keys) across list clients.
  • Strapi mapper TODOs and mocked/contentful localization strings.

Possibly related PRs

Poem

🐇
I hop through rows with gentle cheer,
Check the boxes far and near,
PDFs tumble, one by one,
Selections cleared when work is done,
A rabbit jig for UI fun.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description includes 'What does this PR do?' and 'Key Changes' sections but lacks 'Related Ticket(s)', 'How to test', and 'Media' sections from the template. Add the missing template sections: Related Ticket(s), How to test, and Media (if applicable) to provide complete documentation for reviewers and facilitate testing.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: adding checkboxes in lists' directly summarizes the main change: enabling row selection via checkboxes across multiple list components.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/adding-checkboxes-in-lists

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.invoice-list.mapper.ts (1)

34-36: Pre-existing bug: yesterday is incorrectly mapped to today.

Line 35 sets yesterday: configurableTexts?.dates.today — this appears to be a copy-paste error. It should likely reference configurableTexts?.dates.yesterday.

 labels: {
     today: configurableTexts?.dates.today,
-    yesterday: configurableTexts?.dates.today,
+    yesterday: configurableTexts?.dates.yesterday,
     clickToSelect: configurableTexts.actions.clickToSelect,
 },
packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx (1)

53-69: Missing selection reset when filters change.

Unlike handleReset and the ProductList component, handleFilter does not clear selectedRows. This can leave stale selections referencing items no longer in the current data set.

 const handleFilter = (data: Partial<Request.GetInvoiceListBlockQuery>) => {
     startTransition(async () => {
         try {
             const newFilters = { ...filters, ...data };
             const newData = await sdk.blocks.getInvoiceList(newFilters, { 'x-locale': locale }, accessToken);

             setFilters(newFilters);
             setData(newData);
+            setSelectedRows(new Set());
         } catch (_error) {
             toast({
                 variant: 'destructive',
                 title: labels.errors.requestError.title,
                 description: labels.errors.requestError.content,
             });
         }
     });
 };
🧹 Nitpick comments (4)
packages/blocks/invoice-list/src/api-harmonization/invoice-list.mapper.ts (1)

32-34: New invoice list config fields mapped cleanly from CMS

Forwarding enableRowSelection, bulkActionsLabel, and downloadAllButtonLabel from cms into InvoiceListBlock keeps the mapper in sync with the CMS and UI expectations. One minor refinement you could consider:

-        enableRowSelection: cms.enableRowSelection,
+        enableRowSelection: cms.enableRowSelection ?? false,

if you want a guaranteed boolean for downstream consumers instead of undefined. Not required if the UI already treats undefined as “off”.

packages/blocks/ticket-list/src/frontend/TicketList.client.tsx (1)

52-52: Row selection state integration is correct; minor optimization to consider

The row-selection wiring is implemented properly:

  • selectedRows is reset on data-changing operations (handleFilter, handleReset), preventing stale selections across pages/filters.
  • getRowKey={(item) => item.id} matches the Set<string | number> element type, so keys are stable.
  • DataList correctly creates new Set instances in both handleSelectAll and handleRowSelect handlers, ensuring React state updates remain reliable.

One optional refinement:

Use lazy initialization for the Set to avoid allocating a new Set() on every render:

-    const [selectedRows, setSelectedRows] = useState<Set<string | number>>(new Set());
+    const [selectedRows, setSelectedRows] = useState<Set<string | number>>(
+        () => new Set(),
+    );

Also applies to: 63-63, 80-80, 220-223

packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx (1)

102-127: Consider user feedback for partial bulk download failures.

The inner catch block logs errors to the console but provides no user feedback when individual downloads fail. Additionally, the outer try-catch (lines 119-125) is unreachable since all errors are caught inside the loop.

Consider accumulating failed IDs and showing a single toast after the loop:

 const handleBulkDownload = async (selectedInvoiceIds: string[]) => {
     if (selectedInvoiceIds.length === 0) return;

     startTransition(async () => {
-        try {
-            for (const invoiceId of selectedInvoiceIds) {
-                try {
-                    const response = await sdk.blocks.getInvoicePdf(invoiceId, { 'x-locale': locale }, accessToken);
-                    Utils.DownloadFile.downloadFile(
-                        response,
-                        data.downloadFileName?.replace('{id}', invoiceId) || `invoice-${invoiceId}.pdf`,
-                    );
-                    await new Promise((resolve) => setTimeout(resolve, 100));
-                } catch (error) {
-                    console.error(`Failed to download invoice ${invoiceId}:`, error);
-                }
+        const failedIds: string[] = [];
+        for (const invoiceId of selectedInvoiceIds) {
+            try {
+                const response = await sdk.blocks.getInvoicePdf(invoiceId, { 'x-locale': locale }, accessToken);
+                Utils.DownloadFile.downloadFile(
+                    response,
+                    data.downloadFileName?.replace('{id}', invoiceId) || `invoice-${invoiceId}.pdf`,
+                );
+                await new Promise((resolve) => setTimeout(resolve, 100));
+            } catch (error) {
+                failedIds.push(invoiceId);
             }
-        } catch (_error) {
+        }
+        if (failedIds.length > 0) {
             toast({
                 variant: 'destructive',
                 title: labels.errors.requestError.title,
-                description: labels.errors.requestError.content,
+                description: `Failed to download ${failedIds.length} invoice(s).`,
             });
         }
     });
 };
packages/ui/src/components/DataList/DataList.stories.tsx (1)

218-291: Align selectedItems keying with DataList row keys

selectedItems is filtered with selectedRows.has(ticket.id || index), while DataList’s default key for these rows is effectively String(ticket.id). If id were ever falsy or non-primitive, this fallback to index would drift from the actual selection keys.

Consider simplifying to rely solely on the same key shape used by DataList, e.g.:

- const selectedItems = sampleTickets.filter((ticket, index) => selectedRows.has(ticket.id || index));
+ const selectedItems = sampleTickets.filter((ticket) => selectedRows.has(ticket.id));

This keeps the story aligned with real selection behavior and avoids subtle mismatches if the data shape changes.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c1b5fba and deffd23.

📒 Files selected for processing (20)
  • packages/blocks/invoice-list/src/api-harmonization/invoice-list.mapper.ts (1 hunks)
  • packages/blocks/invoice-list/src/api-harmonization/invoice-list.model.ts (1 hunks)
  • packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx (7 hunks)
  • packages/blocks/notification-list/src/frontend/NotificationList.client.tsx (4 hunks)
  • packages/blocks/notification-list/src/frontend/NotificationList.types.ts (1 hunks)
  • packages/blocks/order-list/src/frontend/OrderList.client.tsx (4 hunks)
  • packages/blocks/order-list/src/frontend/OrderList.types.ts (1 hunks)
  • packages/blocks/product-list/src/frontend/ProductList.client.tsx (3 hunks)
  • packages/blocks/product-list/src/frontend/ProductList.types.ts (1 hunks)
  • packages/blocks/ticket-list/src/frontend/TicketList.client.tsx (4 hunks)
  • packages/blocks/ticket-list/src/frontend/TicketList.types.ts (1 hunks)
  • packages/framework/src/modules/cms/models/blocks/invoice-list.model.ts (1 hunks)
  • packages/integrations/contentful-cms/src/modules/cms/mappers/blocks/cms.invoice-list.mapper.ts (3 hunks)
  • packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.invoice-list.mapper.ts (3 hunks)
  • packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.invoice-list.mapper.ts (1 hunks)
  • packages/ui/src/components/DataList/DataList.stories.tsx (2 hunks)
  • packages/ui/src/components/DataList/DataList.tsx (4 hunks)
  • packages/ui/src/components/DataList/DataList.types.ts (1 hunks)
  • packages/ui/src/components/DataView/DataView.tsx (1 hunks)
  • packages/ui/src/components/DataView/DataView.types.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-13T15:40:10.528Z
Learnt from: marcinkrasowski
Repo: o2sdev/openselfservice PR: 348
File: packages/blocks/notification-summary/src/frontend/NotificationSummary.renderer.tsx:1-15
Timestamp: 2025-11-13T15:40:10.528Z
Learning: In next-intl 3.0+, hook-style APIs like useLocale(), useTranslations(), and useFormatter() can be used in non-async Server Components without the 'use client' directive. The library provides server-optimized implementations automatically. Only async Server Components need to use the await-based APIs like getLocale() from next-intl/server.

Applied to files:

  • packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx
📚 Learning: 2025-12-03T12:34:20.707Z
Learnt from: marcinkrasowski
Repo: o2sdev/openselfservice PR: 419
File: apps/frontend/package.json:52-52
Timestamp: 2025-12-03T12:34:20.707Z
Learning: In the openselfservice repository, the application does not use Next.js cache components (use cache/cacheComponents), so next-intl compatibility limitations related to those experimental features are not a concern.

Applied to files:

  • packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx
📚 Learning: 2025-11-26T11:57:00.632Z
Learnt from: marcinkrasowski
Repo: o2sdev/openselfservice PR: 411
File: packages/framework/src/modules/cms/models/blocks/product-list.model.ts:29-46
Timestamp: 2025-11-26T11:57:00.632Z
Learning: In the framework layer (packages/framework/src/modules/cms/models/blocks/*.model.ts), block classes like ProductListBlock should NOT include explicit __typename discriminators. The __typename field is added at the API harmonization layer (packages/blocks/*/src/api-harmonization/*.model.ts) where it's needed for discriminated unions. The framework maintains normalized data models without these discriminators.

Applied to files:

  • packages/integrations/contentful-cms/src/modules/cms/mappers/blocks/cms.invoice-list.mapper.ts
  • packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.invoice-list.mapper.ts
  • packages/framework/src/modules/cms/models/blocks/invoice-list.model.ts
🧬 Code graph analysis (4)
packages/ui/src/components/DataList/DataList.stories.tsx (2)
packages/ui/src/components/DataList/DataList.tsx (1)
  • DataList (14-149)
packages/ui/src/components/DataList/DataList.types.ts (1)
  • DataListColumnConfig (97-104)
packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx (2)
packages/ui/src/hooks/use-toast.ts (1)
  • toast (189-189)
packages/ui/src/elements/button.tsx (1)
  • Button (19-19)
packages/ui/src/components/DataView/DataView.tsx (2)
packages/ui/src/components/DataGrid/DataGrid.tsx (1)
  • DataGrid (35-150)
packages/ui/src/components/DataList/DataList.tsx (1)
  • DataList (14-149)
packages/blocks/product-list/src/frontend/ProductList.client.tsx (1)
packages/blocks/product-list/src/sdk/index.ts (1)
  • sdk (24-28)
🔇 Additional comments (20)
packages/blocks/product-list/src/frontend/ProductList.types.ts (1)

10-10: Product list prop surface aligned with row selection pattern

Adding enableRowSelection?: boolean to ProductListProps is consistent with other list blocks and keeps the type ready for when the ProductList client/UI wires it through. No issues from a typing/API perspective.

packages/blocks/order-list/src/frontend/OrderList.types.ts (1)

11-11: Order list now exposes row-selection toggle

enableRowSelection?: boolean on OrderListProps fits the emerging API across list blocks and is non‑breaking. Looks good.

packages/blocks/ticket-list/src/frontend/TicketList.types.ts (1)

12-12: Ticket list props correctly extended for row selection

The enableRowSelection?: boolean flag is well‑placed alongside other feature toggles and matches its use in TicketList.client.tsx. No changes needed.

packages/blocks/notification-list/src/frontend/NotificationList.types.ts (1)

11-11: Notification list props aligned with selection-enabled lists

enableRowSelection?: boolean mirrors the other list blocks and matches its consumption in NotificationList.client.tsx. Typing looks correct.

packages/integrations/contentful-cms/src/modules/cms/mappers/blocks/cms.invoice-list.mapper.ts (1)

117-119: Invoice list CMS mocks consistently configured for bulk selection

The new mock fields

  • enableRowSelection: true
  • bulkActionsLabel with {count, plural, ...}
  • downloadAllButtonLabel

are applied consistently across EN/DE/PL and the plural forms look appropriate for each locale. This should give the UI everything it needs for localized bulk‑selection UX.

Also applies to: 236-238, 355-358

packages/blocks/notification-list/src/frontend/NotificationList.client.tsx (1)

55-55: Notification row selection wired consistently with other lists

The notification list mirrors the TicketList pattern:

  • selectedRows stored in state as a Set<string | number>.
  • Selection cleared on handleFilter and handleReset, so it doesn’t leak across result sets.
  • enableRowSelection={component.enableRowSelection} plus getRowKey={(item) => item.id} matches the new public prop and keeps keying straightforward.

Implementation looks correct and consistent with the rest of the PR.

Also applies to: 66-66, 88-88, 207-210

packages/blocks/invoice-list/src/api-harmonization/invoice-list.model.ts (1)

33-35: LGTM!

The new optional properties for row selection and bulk actions are well-defined and align with the framework model and CMS mapper changes across the PR.

packages/blocks/product-list/src/frontend/ProductList.client.tsx (3)

38-38: LGTM!

Good use of Set<string | number> for tracking selected rows, consistent with the selection pattern established across other list components.


46-46: Selection reset on data changes is correctly implemented.

Clearing selectedRows when filters are applied or reset prevents stale selections from persisting after the data set changes.

Also applies to: 55-55


168-170: enableRowSelection is properly declared in ProductList.types.ts.

The property is defined at line 10 as enableRowSelection?: boolean;, confirming it is sourced from props. No action needed.

packages/framework/src/modules/cms/models/blocks/invoice-list.model.ts (1)

30-32: LGTM!

The new properties are correctly added as optional fields without __typename discriminators, aligning with the framework layer conventions. Based on learnings, __typename is appropriately added only at the API harmonization layer.

packages/ui/src/components/DataList/DataList.types.ts (1)

154-168: LGTM!

The row selection API follows the controlled component pattern correctly with selectedRows as the value and onSelectionChange as the callback. The JSDoc comments clearly document the purpose of each prop.

packages/integrations/strapi-cms/src/modules/cms/mappers/blocks/cms.invoice-list.mapper.ts (1)

42-44: LGTM - TODO placeholders are appropriate.

Setting these to undefined with TODO comments is a reasonable approach for staged rollout while the CMS schema is being updated.

packages/ui/src/components/DataView/DataView.types.ts (1)

21-26: LGTM!

The new props extend DataViewProps consistently with DataListProps. The render prop pattern for bulkActions and function-based bulkActionsLabel provide good flexibility for customization and localization.

packages/blocks/invoice-list/src/frontend/InvoiceList.client.tsx (2)

191-208: LGTM!

Good use of IntlMessageFormat for ICU message formatting with locale awareness. The conditional definitions for bulkActions and bulkActionsLabel correctly handle the case when these features are disabled.


249-254: Row selection props are correctly wired to DataView.

The controlled selection state pattern with enableRowSelection, selectedRows, and onSelectionChange is properly implemented.

packages/ui/src/components/DataList/DataList.stories.tsx (1)

2-5: Imports are consistent with new stories

lucide-react, React/useState, and Button are all used in the new stories; the imports look correct with no obvious redundancy.

packages/blocks/order-list/src/frontend/OrderList.client.tsx (1)

55-85: Row selection state wiring looks sound (depends on DataView fix)

Using Set<string | number> for selectedRows, resetting it on filter apply/reset, and passing enableRowSelection, selectedRows, onSelectionChange, and getRowKey={(item) => item.id.value} into DataView is a solid pattern for controlled selection.

Just note this relies on DataView actually forwarding getRowKey down to DataList/DataGrid; see the corresponding comment in DataView.tsx where this isn’t currently happening.

Also applies to: 214-218

packages/ui/src/components/DataList/DataList.tsx (1)

3-3: Row selection implementation is consistent and controlled

The added selection API (enableRowSelection, selectedRows, onSelectionChange) together with handleSelectAll/handleRowSelect and the header/row checkboxes form a coherent controlled selection model:

  • Select‑all operates only on the current page’s keys.
  • Per‑row toggles clone the Set before updating, which plays nicely with React state.
  • When selection props are omitted, the component safely no‑ops instead of throwing.

Looks good and matches the needs of DataView and the new stories.

Also applies to: 21-23, 35-72, 77-85, 108-108, 112-120

packages/integrations/mocked/src/modules/cms/mappers/blocks/cms.invoice-list.mapper.ts (1)

135-137: Invoice-list mock config extended consistently for bulk selection

enableRowSelection, bulkActionsLabel, and downloadAllButtonLabel are added for EN/DE/PL with consistent semantics and appropriate ICU pluralization patterns. This should integrate cleanly with the new bulk‑actions UI and localization pipeline.

Also applies to: 272-274, 409-412

Comment thread packages/ui/src/components/DataView/DataView.tsx
@vercel vercel Bot temporarily deployed to Preview – o2s-docs December 18, 2025 11:16 Inactive
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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

🧹 Nitpick comments (2)
packages/ui/src/components/Cards/PricingCard/PricingCard.tsx (1)

9-9: Consider moving unrelated import refactoring to a separate PR.

This import path change (from relative to package-based) is a reasonable standardization but appears unrelated to the PR's stated objective of "adding checkboxes in lists." The PricingCard component isn't involved in list selection or bulk actions.

Keeping unrelated refactoring separate improves PR clarity, review focus, and git history.

packages/ui/src/components/DataList/DataList.tsx (1)

70-71: Remove redundant length check.

Line 71 checks both data.length > 0 and allRowKeys.length > 0. Since allRowKeys is derived directly from data.map(), these lengths are always identical, making the second check redundant.

🔎 Simplify the condition:
     const allSelected =
-        enableRowSelection && data.length > 0 && selectedOnCurrentPage === allRowKeys.length && allRowKeys.length > 0;
+        enableRowSelection && data.length > 0 && selectedOnCurrentPage === allRowKeys.length;
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6290a35 and bc9e361.

📒 Files selected for processing (8)
  • packages/ui/src/components/Cards/InformativeCard/InformativeCard.tsx (1 hunks)
  • packages/ui/src/components/Cards/InformativeCard/InformativeCard.types.ts (1 hunks)
  • packages/ui/src/components/Cards/PricingCard/PricingCard.tsx (1 hunks)
  • packages/ui/src/components/DataGrid/DataGrid.stories.tsx (1 hunks)
  • packages/ui/src/components/DataGrid/DataGrid.tsx (1 hunks)
  • packages/ui/src/components/DataList/DataList.stories.tsx (2 hunks)
  • packages/ui/src/components/DataList/DataList.tsx (4 hunks)
  • packages/ui/src/components/LinkList/LinkList.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/ui/src/components/DataList/DataList.stories.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: deploy-preview
🔇 Additional comments (9)
packages/ui/src/components/Cards/InformativeCard/InformativeCard.types.ts (1)

3-3: LGTM! Import path refactored to package-scoped pattern.

The change from relative to package-scoped import aligns with the existing import style in this file and improves maintainability.

packages/ui/src/components/Cards/InformativeCard/InformativeCard.tsx (1)

7-7: LGTM! Import path standardized to match other components.

Refactoring the RichText import to use the package-scoped path is consistent with the DynamicIcon import on line 6 and improves code maintainability.

packages/ui/src/components/LinkList/LinkList.tsx (1)

5-7: Clean refactoring to shared modules.

The import consolidation to @o2s/ui/components/DynamicIcon and @o2s/ui/elements/link aligns with the PR's standardization effort. Both modules exist and correctly export their respective components.

packages/ui/src/components/DataGrid/DataGrid.tsx (1)

3-6: LGTM! Import paths standardized.

The import paths have been updated to use centralized @o2s/ui package paths instead of relative imports, improving maintainability and consistency across the codebase.

packages/ui/src/components/DataGrid/DataGrid.stories.tsx (1)

4-6: LGTM! Improved TypeScript imports.

The imports have been standardized to use @o2s/ui paths, and the type keyword is correctly used for the type-only import of DataListColumnConfig, which is a TypeScript best practice that helps with tree-shaking.

packages/ui/src/components/DataList/DataList.tsx (4)

22-24: LGTM! Row selection props properly defined.

The new props enableRowSelection, selectedRows, and onSelectionChange are correctly typed and support both controlled and uncontrolled selection patterns. The default value of false for enableRowSelection ensures backward compatibility.


37-52: LGTM! Select-all logic correctly handles pagination.

The handleSelectAll function properly toggles selection for all rows on the current page while preserving selections from other pages. The implementation safely handles undefined selectedRows by defaulting to an empty array.


78-86: LGTM! Header checkbox properly implements tri-state selection.

The header checkbox correctly handles three states (all selected, indeterminate, none selected) and includes proper accessibility labels. The implementation follows expected table selection patterns.


113-121: LGTM! Row checkbox implementation is correct.

The per-row checkbox properly tracks selection state and includes defensive type coercion (!!checked) to ensure a boolean value is passed to handleRowSelect. Accessibility is appropriately handled with aria-label.

Comment thread packages/ui/src/components/DataList/DataList.tsx
Comment thread packages/ui/src/components/DataList/DataList.tsx
@vercel vercel Bot temporarily deployed to Preview – o2s-docs December 18, 2025 11:22 Inactive
@vercel vercel Bot temporarily deployed to Preview – o2s-docs December 18, 2025 11:39 Inactive
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a 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

♻️ Duplicate comments (2)
packages/ui/src/components/DataList/DataList.tsx (2)

55-65: Handler asymmetry resolved.

The previous concern about handleRowSelect failing when selectedRows is undefined has been addressed. Line 58 now correctly initializes from selectedRows || [], matching the pattern in handleSelectAll.


67-76: Expensive operations are now memoized; consider completing the optimization.

The critical performance concern from the previous review has been addressed—allRowKeys (line 68) and selectedOnCurrentPage (lines 70–73) are now memoized, preventing expensive map and filter operations on every render.

However, allSelected and someSelected (lines 74–76) remain unmemoized. While these are cheap boolean comparisons with minimal overhead, wrapping all four values in a single useMemo as originally suggested would provide consistency and eliminate any remaining recalculation.

🧹 Nitpick comments (1)
packages/blocks/invoice-list/src/frontend/InvoiceList.client.stories.tsx (1)

14-324: Consider adding the new row selection properties to the story.

The PR introduces enableRowSelection, bulkActionsLabel, and downloadAllButtonLabel properties to the InvoiceListBlock. Adding these to the story args would help demonstrate and visually test the new bulk actions feature.

🔎 Suggested addition to the args object:
         downloadFileName: 'invoice-{id}.pdf',
         downloadButtonAriaDescription: 'Download invoice {id}',
+        enableRowSelection: true,
+        bulkActionsLabel: '{count} invoices selected',
+        downloadAllButtonLabel: 'Download selected',
     },
 };
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bc9e361 and 973ad93.

📒 Files selected for processing (4)
  • packages/blocks/invoice-list/src/frontend/InvoiceList.client.stories.tsx (5 hunks)
  • packages/ui/src/components/DataGrid/DataGrid.tsx (1 hunks)
  • packages/ui/src/components/DataList/DataList.tsx (4 hunks)
  • packages/ui/src/lib/renderCell.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/ui/src/components/DataGrid/DataGrid.tsx
  • packages/ui/src/lib/renderCell.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/ui/src/components/DataList/DataList.tsx (2)
packages/ui/src/elements/table.tsx (5)
  • Table (64-64)
  • TableHeader (64-64)
  • TableRow (64-64)
  • TableHead (64-64)
  • TableCell (64-64)
packages/ui/src/elements/checkbox.tsx (1)
  • Checkbox (99-99)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: deploy-preview
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
packages/blocks/invoice-list/src/frontend/InvoiceList.client.stories.tsx (1)

176-183: LGTM!

The addition of currency fields to the nested totalAmountDue and totalNetAmountDue objects aligns the mock data with the updated type definitions. The changes are consistent across all invoice entries.

packages/ui/src/components/DataList/DataList.tsx (5)

1-6: All imports are properly utilized.

All imported modules are actively used in the component implementation.


23-25: Sensible defaults for selection props.

The enableRowSelection default of false ensures backward compatibility, and making selection props optional allows flexible adoption.


37-53: Select-all logic correctly preserves multi-page selection state.

The implementation properly adds or removes only the current page's keys, preserving selections from other pages.


82-90: Select-all checkbox correctly implements tri-state logic.

The header checkbox properly displays indeterminate state when some (but not all) rows are selected, and includes appropriate accessibility labels.


113-125: Row selection checkboxes properly integrated.

The row-level checkboxes correctly reflect selection state and handle user interaction. The !!checked conversion on line 121 is defensive but harmless, ensuring boolean type safety.

@marcinkrasowski marcinkrasowski merged commit d197b89 into main Dec 18, 2025
9 checks passed
@marcinkrasowski marcinkrasowski deleted the feature/adding-checkboxes-in-lists branch December 18, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants