[DREAM-688] Use BorderBoxList for enumerations#23953
Open
myabc wants to merge 1 commit into
Open
Conversation
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
myabc
added
needs review
ruby
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the Administration → Enumerations index list to OpenProject::Common::BorderBoxListComponent, aligning the admin UI with the shared BorderBox list composition while keeping drag-and-drop reordering behavior intact.
Changes:
- Replaced bespoke
border_box_containermarkup in the enumerations index withBorderBoxListComponentheader/item/empty-state slots. - Added component spec coverage for the enumerations index, including empty-state rendering and DnD-related data attributes.
- Introduced shared RSpec examples for BorderBoxListComponent list headings, empty states, and generic reorderable-list behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| spec/support/shared/components/border_box_list_component.rb | Adds shared examples for BorderBoxListComponent heading/empty state and reorderable-list expectations. |
| spec/components/admin/enumerations/index_component_spec.rb | Adds component spec coverage for Admin::Enumerations::IndexComponent (rows, empty state, DnD attributes). |
| app/components/admin/enumerations/index_component.html.erb | Switches enumerations index rendering to BorderBoxListComponent slots (header/items/empty state). |
Comment on lines
+75
to
+88
| it "renders each record as a draggable row pointing at its drop URL", :aggregate_failures do | ||
| draggable_records.each do |record| | ||
| selector = ".Box-row[data-draggable-type='#{drag_type}'][data-draggable-id='#{draggable_id_for(record)}']" | ||
|
|
||
| expect(rendered_component).to have_css(selector) do |row| | ||
| expect(row["data-drop-url"]).to end_with(drop_url_for(record)) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def draggable_id_for(record) | ||
| record.id | ||
| end | ||
| end |
| let!(:priority_b) { create(:priority, name: "Trivial") } | ||
| let(:enumerations) { IssuePriority.where(id: [priority_a.id, priority_b.id]).order(:position) } | ||
|
|
||
| it_behaves_like "rendering Box", row_count: 2 |
Comment on lines
+52
to
+65
| it "renders a drag-and-drop reorderable container", :aggregate_failures do | ||
| expect(rendered_component) | ||
| .to have_css(".Box.op-border-box-list[data-generic-drag-and-drop-target='container']") do |box| | ||
| expect(box["data-target-container-accessor"]).to eq(":scope > ul") | ||
| expect(box["data-target-allowed-drag-type"]).to eq("enumeration") | ||
| end | ||
|
|
||
| [priority_a, priority_b].each do |enumeration| | ||
| expect(rendered_component).to have_css( | ||
| ".Box-row[data-draggable-type='enumeration']" \ | ||
| "[data-draggable-id='#{enumeration.id}'][data-drop-url]" | ||
| ) | ||
| end | ||
| end |
|
Warning Flaky specs
🤖 Ask Copilot to investigateCopy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
https://community.openproject.org/wp/DREAM-688
What are you trying to accomplish?
Migrate the enumerations admin index list to the shared
OpenProject::Common::BorderBoxListComponent, validating the primitive in a real drag-and-drop reordering context. Header, rows, empty state, and DnD wiring are preserved and covered by a component spec that shares the"a reorderable Border Box List"example.Note
Document Types are not migrated in this PR. Since the page actually uses a semantic table, it is intentionally left out of scope here.
Screenshots
What approach did you choose and why?
Replaced the bespoke
border_box_containerheader/row markup with the list component's slots (with_header,with_item,with_empty_state). The header now renders as anh3to match sibling admin lists, and the itemless list shows the default Blankslate empty state. Drag target and per-row draggable data attributes, the turbo wrapper, and locale keys are preserved.The markup and specs are kept byte-identical to the equivalent commit in the wider DREAM-697 migration (#23812) so the two branches reconcile without conflict.
Merge checklist