[#74684] Extract BorderBoxListComponent#23074
Conversation
There was a problem hiding this comment.
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::WorkPackageCardBoxComponenttoOpenProject::Common::WorkPackageCardListComponent(including nestedHeader/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_stateslot still talks about “items” / “automatic item builds”, but the component now renders rows directly fromwork_packages(and validates based onwork_packages.empty?). Please update the comment to match the current behavior so it stays accurate.
EinLama
left a comment
There was a problem hiding this comment.
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.
20fd06f to
51ebb2f
Compare
WorkPackageCardListComponent, Extract BorderBoxListComponent
3005f6e to
1a753d1
Compare
There was a problem hiding this comment.
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_componentto...work_package_card_list_componentupdates onlyen.yml. All Crowdin locale files still definework_package_card_box_componentand have nowork_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"
1a753d1 to
a280bbb
Compare
WorkPackageCardListComponent, Extract BorderBoxListComponentBorderBoxListComponent
a280bbb to
f25a281
Compare
dcce7f5 to
7ef28be
Compare
f25a281 to
44d2dde
Compare
|
|
||
| module OpPrimer | ||
| class BorderBoxListComponent | ||
| module Menu |
There was a problem hiding this comment.
Let's make this a concern. Also rename to following concern naming conventions, e.g. HasMenu
44d2dde to
e46b196
Compare
Add the generic OpPrimer BorderBox list shell and the OpenProject::Common wrapper for work-package card rows. Move shared header, menu, item, empty-state, footer, and container-derived id handling into the list components, keep work-package-specific row support out of OpPrimer, and update backlogs callers and component specs.
e46b196 to
39b5174
Compare
|
|
||
| See COPYRIGHT and LICENSE files for more details. | ||
|
|
||
| ++ %> |
| def with_header(title:, count: nil, &) | ||
| @list.with_header( | ||
| title:, | ||
| count:, | ||
| collapsed: folded?, | ||
| count_aria_label: count ? t(".header.label_work_package_count", count:) : nil, | ||
| id: header_id, | ||
| & | ||
| ) |
Moves `HasMenu`, `Header`, `Item`, and `Footer` from `OpPrimer::BorderBoxListComponent` into the `OpenProject::Common` namespace. `Item` is simplified: `HasMenu` is dropped from generic items per the v2 design.
`EmptyState` renders a `Primer::Beta::Blankslate` for empty lists. `WorkPackageItem` is the base WP row bridge, adapted from `WorkPackageCardListComponent::Item`.
Renders `Primer::Beta::BorderBox` directly instead of delegating to `OpPrimer::BorderBoxListComponent`. Adds `container:` (required) and `current_user:` to the constructor, polymorphic items with `with_work_package_item`, and `with_empty_state` (Blankslate).
Moves header grid layout from the OpPrimer sass. Item grid styles are dropped since generic items no longer have a menu sub-slot.
The auto-collection wrapper is internal to Backlogs and not yet ready for sharing. Hardcodes `WorkPackageCardListItemComponent` during auto-iteration and drops the `item_component_klass:` param. Composes the new `BorderBoxListComponent` instead of the old list. Updates the item component superclass to `WorkPackageItem` accordingly.
`InboxComponent` uses `BorderBoxListComponent` directly with the new `with_empty_state` slot and per-item `component_klass:`. `SprintComponent` and `BucketComponent` switch from the old `OpenProject::Common::WorkPackageCardListComponent` to the new `Backlogs::WorkPackageCardListComponent` wrapper.
Covers the new constructor (`container:`, `current_user:`), polymorphic items, `with_empty_state`, `WorkPackageItem` injection, and container-derived DOM IDs.
Covers auto-iteration, fold-state header, drag-and-drop data, empty state validation, Blankslate rendering, delegated footer, and DOM ID derivation from container. Verifies items render through `Backlogs::WorkPackageCardListItemComponent`. Item component spec unchanged: no references to the old superclass name.
Covers header variants, generic items, work-package items (default `WorkPackageItem`), empty state, and footer.
Deletes `OpPrimer::BorderBoxListComponent` namespace, the old `OpenProject::Common::WorkPackageCardListComponent`, their specs, and Lookbook previews. All coverage now lives in the new single-layer `BorderBoxListComponent` and `Backlogs::WorkPackageCardListComponent`. Removes the corresponding SASS imports from `_index.sass`.
Note
This PR is based on #23146. Please review/merge that PR first.
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 rendersPrimer::Beta::BorderBoxdirectly. Providesheader,with_item,with_work_package_item,with_empty_state, andfooterslots. Owns container-derived DOM IDs. Supports manual item composition (callers control render order).Backlogs::WorkPackageCardListComponent— An internal auto-collection wrapper. Acceptswork_packages:, iterates the collection, handles empty-state and drag-and-drop. Moved toBacklogssince it's not yet ready for sharing with other teams.Backlogs
InboxComponentusesBorderBoxListComponentdirectly (manual composition for show-more interleaving).SprintComponentandBucketComponentuse the Backlogs wrapper.What approach did you choose and why?
BorderBoxListComponentrendersPrimer::Beta::BorderBoxdirectly with polymorphic item slots — generic content rows and work-package rows in the same collection.WorkPackageItemis a base class nested underBorderBoxListComponentthat Backlogs subclasses for drag-and-drop and story-specific behavior.Merge checklist