[#74684] Extract BorderBoxListComponent

[myabc]

Ticket

https://community.openproject.org/wp/74684

What are you trying to accomplish?

Extracts a reusable BorderBox-backed list component out of WorkPackageCardListComponent (renamed in #23146), splitting the responsibility across two layers:

• OpenProject::Common::BorderBoxListComponent — A domain-aware list that renders Primer::Beta::BorderBox directly. Provides header, with_item, with_work_package_item, with_empty_state, and footer slots. Owns container-derived DOM IDs. Supports manual item composition (callers control render order).

• Backlogs::WorkPackageCardListComponent — An internal auto-collection wrapper. Accepts work_packages:, iterates the collection, handles empty-state and drag-and-drop. Moved to Backlogs since it's not yet ready for sharing with other teams.

Backlogs InboxComponent uses BorderBoxListComponent directly (manual composition for show-more interleaving). SprintComponent and BucketComponent use the Backlogs wrapper.

What approach did you choose and why?

BorderBoxListComponent renders Primer::Beta::BorderBox directly with polymorphic item slots — generic content rows and work-package rows in the same collection. WorkPackageItem is a base class nested under BorderBoxListComponent that Backlogs subclasses for drag-and-drop and story-specific behavior.

Also switches Backlogs templates from data: { test_selector: ... } to the test_selector: Primer system argument.

Merge checklist

• Added/updated tests
• Added/updated documentation
  • Lookbook previews
  • YARDDoc
• Tested major browsers (Chrome, Firefox, Edge, ...)

[Copilot]

Pull request overview

This PR completes a rename/refactor of the work package “card box” UI into a WorkPackageCardListComponent, updates Backlogs to use the renamed component/item class, and introduces a reusable OpPrimer::BorderBoxListComponent wrapper to standardize BorderBox list rendering (header/rows/footer/empty rows).

Changes:

• Rename OpenProject::Common::WorkPackageCardBoxComponent to OpenProject::Common::WorkPackageCardListComponent (including nested Header/Item) and update associated styles and I18n keys.
• Add OpPrimer::BorderBoxListComponent (with content/empty/work-package row bridges) and wire the work package card list to render via this component.
• Update Backlogs (bucket/sprint/inbox) rendering and specs to use the new list + item components and updated accessibility labels.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
spec/components/open_project/common/work_package_card_list_component/item_spec.rb	Updates spec namespace for the renamed Item component and related test doubles.
spec/components/open_project/common/work_package_card_list_component/header_spec.rb	Updates spec namespace for the renamed Header component.
spec/components/open_project/common/work_package_card_list_component_spec.rb	Updates spec namespace for the renamed list component and removes coverage for removed manual-slot behavior.
spec/components/op_primer/border_box_list_component_spec.rb	Adds unit coverage for the new OpPrimer::BorderBoxListComponent behavior.
modules/backlogs/spec/support/pages/backlog.rb	Updates I18n key for counter accessible label; minor Capybara matcher adjustment.
modules/backlogs/spec/components/backlogs/work_package_card_list_item_component_spec.rb	Updates spec to the renamed Backlogs item component.
modules/backlogs/spec/components/backlogs/bucket_component_spec.rb	Updates I18n key used for the counter aria-label.
modules/backlogs/app/components/backlogs/work_package_card_list_item_component.rb	Renames Backlogs work package item component to inherit from the renamed common list item.
modules/backlogs/app/components/backlogs/sprint_component.html.erb	Switches sprint rendering to WorkPackageCardListComponent and renamed Backlogs item component.
modules/backlogs/app/components/backlogs/inbox_component.html.erb	Reworks inbox rendering to use OpPrimer::BorderBoxListComponent directly and updates CSS class name.
modules/backlogs/app/components/backlogs/bucket_component.html.erb	Switches bucket rendering to WorkPackageCardListComponent and renamed Backlogs item component.
lookbook/previews/open_project/common/work_package_card_list_component_preview.rb	Renames preview class and updates it to render the renamed list component; removes manual-item preview.
lookbook/previews/op_primer/border_box_list_component_preview.rb	Adds Lookbook preview coverage for the new BorderBoxListComponent.
config/locales/en.yml	Renames translation namespace from work_package_card_box_component to work_package_card_list_component.
app/components/open_project/common/work_package_card_list_component/item.rb	Renames outer component constant for the nested Item and keeps item row bridging behavior.
app/components/open_project/common/work_package_card_list_component/header.sass	Renames header CSS class from ...card-box... to ...card-list....
app/components/open_project/common/work_package_card_list_component/header.rb	Renames outer component constant for the nested Header.
app/components/open_project/common/work_package_card_list_component/header.html.erb	Updates header wrapper class to the renamed CSS class.
app/components/open_project/common/work_package_card_list_component.sass	Renames base CSS class from ...card-box... to ...card-list....
app/components/open_project/common/work_package_card_list_component.rb	Renames the component class and simplifies rendering/validation to “work_packages-driven” list rendering.
app/components/open_project/common/work_package_card_list_component.html.erb	Adds a dedicated template that renders via OpPrimer::BorderBoxListComponent.
app/components/op_primer/border_box_list_component/work_package_item.rb	Replaces incorrect/spec-like content with the actual WorkPackageItem bridge implementation.
app/components/op_primer/border_box_list_component/empty_item.rb	Moves/implements EmptyItem bridge under OpPrimer::BorderBoxListComponent.
app/components/op_primer/border_box_list_component/content_item.rb	Moves/implements ContentItem bridge under OpPrimer::BorderBoxListComponent.
app/components/op_primer/border_box_list_component.rb	Introduces the new BorderBox list wrapper component API.
app/components/op_primer/border_box_list_component.html.erb	Updates rendering to use rows and render header/footer via stored bridge objects.
app/components/_index.sass	Updates Sass imports to the renamed work package card list component styles.

Comments suppressed due to low confidence (2)

app/components/open_project/common/work_package_card_list_component/item.rb:36

• The class-level comment still refers to a “card box”, but the constant is now WorkPackageCardListComponent. Please update the wording (and any similar occurrences) to avoid confusion for future readers and to reflect the renamed component accurately.
  app/components/open_project/common/work_package_card_list_component.rb:140
• The surrounding documentation for the empty_state slot still talks about “items” / “automatic item builds”, but the component now renders rows directly from work_packages (and validates based on work_packages.empty?). Please update the comment to match the current behavior so it stays accurate.

[EinLama]

This change makes sense to me 👍

It would be good to wait for the next merge of the release branch and apply the removal of the mirrorContainer here, too.

[Copilot]

Pull request overview

Copilot reviewed 37 out of 37 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

config/locales/en.yml:4913

• Renaming the I18n scope from open_project.common.work_package_card_box_component to ...work_package_card_list_component updates only en.yml. All Crowdin locale files still define work_package_card_box_component and have no work_package_card_list_component, so non-English locales will start showing missing translations (or fallback to English) for the header count/labels. Consider keeping the old key as an alias (temporarily) and/or updating the Crowdin locale keys as part of this change.

      work_package_card_list_component:
        header:
          label_actions: "Open menu"
          label_work_package_count:
            zero: "No work packages"
            one: "%{count} work package"
            other: "%{count} work packages"

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 27 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated 1 comment.

[bsatarnejad]

Should every empty state be rendered as role="status" with aria-live="polite" by default?

It might be more announcement-heavy than needed. Maybe this should be configurable, with the current behavior as an option?

[bsatarnejad]

Should the counter always get aria-live="polite" by default?

For static initial rendering this may be unnecessary, and in Backlogs we may render several counters at once. Would it be safer to make the live behavior opt-in, or only apply it where the counter is expected to update dynamically?

[bsatarnejad]

Is this new file imported into the compiled frontend styles. Could we import it?

[bsatarnejad]

@system_arguments[:list_id] = dom_target(...) unconditionally overwrites any caller-provided list_id:. The spec ("derives the list id from the explicit box id") even asserts that an explicit list_id: is ignored. Same for footer id: at line 168 — the Footer stores the original in attr_reader :id but the rendered id is always the derived one. If these aren't meant to be caller-controlled, consider filtering them out or documenting they're reserved, rather than silently discarding them.

[myabc]

deny_single_argument would make this easy to implement, but we would have to inherit from Primer::Component instead.

[bsatarnejad]

The old WorkPackageCardListComponent header never set a default button scheme, and sprint_component.rb explicitly passed scheme: :invisible on the start/finish sprint buttons. But here we remove those scheme: :invisible overrides, so the sprint buttons now render with :default scheme instead of :invisible. This is a user-visible UI change. Is it intentional? If so, worth calling out in the PR description.

[myabc]

Yes, it's intentional. It's preparatory work for container header restyling in #23215 / OP #72945. I should have mentioned this small visual change in the PR description.

[bsatarnejad]

The default label is intentionally generic ("5 items"). The Backlogs wrapper compensates by passing :label_x_work_packages. A short inline comment noting "callers should supply count_label: when a domain-specific label is needed" would help future consumers of this generic component avoid accidentally shipping "5 items" when they meant "5 work packages" (or similar).

[myabc]

@bsatarnejad count_label is documented on Header#initialize. Is this not enough?

        # @param count_label [String, nil] accessible label for the counter
        #   badge. Defaults to `I18n.t(:label_x_items, count:)` when a count
        #   is rendered. Pass an explicit string to override.

[bsatarnejad]

The old header template always rendered the menu grid area. This now conditionally renders it with if menu? This is cleaner, but worth confirming no CSS or JS relied on the .op-border-box-list-header--menu area always being present in the grid layout.

[bsatarnejad]

with_description { @description } works but the old code used with_description_content(description), which is the Primer documented API for passing a plain string. The block form is fine but slightly inconsistent with the chaining style used for with_heading two lines above.