Skip to content

[DREAM-688] Use BorderBoxList for enumerations - #23485

Closed
myabc wants to merge 2 commits into
devfrom
implementation/74940-migrate-enumerations-to-border-box-list
Closed

[DREAM-688] Use BorderBoxList for enumerations#23485
myabc wants to merge 2 commits into
devfrom
implementation/74940-migrate-enumerations-to-border-box-list

Conversation

@myabc

@myabc myabc commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Ticket

https://community.openproject.org/wp/DREAM-688

What are you trying to accomplish?

Migrate the enumerations and document-types admin index lists to the shared OpenProject::Common::BorderBoxListComponent, validating the primitive in real drag-and-drop reordering contexts. Header, rows, empty state, and DnD wiring are preserved and covered by component specs that share a "a reorderable Border Box List" example.

Screenshots

What approach did you choose and why?

Replaced the bespoke border_box_container header/row markup with the list component's slots (with_header, with_item, with_empty_state). The document-types per-row document count now renders as a Counter using the same options as the list header counter.

Merge checklist

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

@myabc
myabc requested a review from Copilot June 1, 2026 10:58
@myabc myabc added this to the 17.6.x milestone Jun 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the admin enumerations index to render its list using the shared OpenProject::Common::BorderBoxListComponent, while adding a focused component spec to keep drag-and-drop-related markup covered by tests.

Changes:

  • Replaces the hand-rolled Primer::Beta::BorderBox composition with OpenProject::Common::BorderBoxListComponent in the enumerations index view.
  • Adds a component spec ensuring the BorderBox list markup, controls, and drag-and-drop data attributes are present for both empty and non-empty states.

Reviewed changes

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

File Description
app/components/admin/enumerations/index_component.html.erb Switches enumeration list rendering to the shared BorderBoxList component while keeping drag-and-drop data attributes.
spec/components/admin/enumerations/index_component_spec.rb Adds component-level coverage for header/actions, row rendering, and drag-and-drop dataset attributes.

Comment thread app/components/admin/enumerations/index_component.html.erb
@myabc myabc changed the title [#74940] Use BorderBoxList for enumerations [SDS-688] Use BorderBoxList for enumerations Jun 2, 2026
@myabc
myabc force-pushed the implementation/74940-migrate-enumerations-to-border-box-list branch from 22a29ff to 70873c9 Compare June 16, 2026 18:48
@myabc myabc changed the title [SDS-688] Use BorderBoxList for enumerations [DREAM-688] Use BorderBoxList for enumerations Jun 16, 2026
@myabc
myabc marked this pull request as ready for review June 16, 2026 18:56
@myabc
myabc requested a review from Copilot June 16, 2026 18:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@myabc
myabc force-pushed the implementation/74940-migrate-enumerations-to-border-box-list branch from 70873c9 to 5822734 Compare June 16, 2026 19:01
myabc added 2 commits June 16, 2026 21:09
Migrate the enumerations admin index from hand-rolled BorderBox header
and row markup to the shared BorderBoxListComponent, exercising the
primitive in a real drag-and-drop reordering context. The header now
renders as an h3 to match sibling admin lists. Adds spec coverage.

https://community.openproject.org/wp/DREAM-688
Migrate the document types admin index to BorderBoxListComponent,
mirroring the enumerations migration. The old two-column header
(Type / Documents) is replaced by the list header with a type count;
each row renders its document count as a Counter using the same
options as the list header counter, minus the primary scheme.
Adds spec coverage.

https://community.openproject.org/wp/DREAM-688
@myabc
myabc force-pushed the implementation/74940-migrate-enumerations-to-border-box-list branch from 5822734 to 14c37c0 Compare June 16, 2026 21:05
@myabc myabc added needs review ruby Pull requests that update Ruby code styling and removed DO NOT MERGE labels Jun 16, 2026

@HDinger HDinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code changes themeselves look fine. However, I have two visual remarks:

  • I am not convinced by the decision to get rid of the second column in the documents table. Please bring that to the UX/UI and also reflect that in the ticket if it gets agreement
  • The "default" badge looks different in the two tables. I am aware that this was not necessarily part of the change, but I guess it makes sense to harmonize that quickly to use the Label in both places
Image Image

@myabc

myabc commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favour of #23812

@myabc myabc closed this Jun 19, 2026
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 19, 2026
@opf opf unlocked this conversation Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review ruby Pull requests that update Ruby code styling

Development

Successfully merging this pull request may close these issues.

3 participants