[OP-19305] Enforce order of subheader slots/quickfilters - #469
Conversation
🦋 Changeset detectedLatest commit: b951013 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
There was a problem hiding this comment.
Pull request overview
This PR introduces dedicated sort_quick_filter and group_quick_filter slots to Primer::OpenProject::SubHeader so their rendering order is enforced ahead of the existing quick_filters collection, and updates previews/tests accordingly.
Changes:
- Added
with_sort_quick_filterandwith_group_quick_filterslots and enforced their render order before other quick filters. - Updated
before_renderlogic so sort/group count toward the max quick-filter limit, but do not affect the filter-button requirement. - Added component tests and updated the Lookbook preview; included a changeset for the new API.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
app/components/primer/open_project/sub_header.rb |
Adds sort_quick_filter/group_quick_filter slots and updates quick-filter counting logic. |
app/components/primer/open_project/sub_header.html.erb |
Renders sort/group slots before the existing quick_filters loop. |
test/components/primer/open_project/sub_header_test.rb |
Adds coverage for the new slots and ordering/counting behavior. |
previews/primer/open_project/sub_header_preview/quick_filters.html.erb |
Demonstrates the new sort/group quick-filter slots in the preview. |
.changeset/empty-poets-occur.md |
Declares a minor release for the new slots. |
…he order is enforced
69f4c16 to
d51cc25
Compare
myabc
left a comment
There was a problem hiding this comment.
Looks good 👍🏻
I have a couple suggestions, but they are not blockers:
- naming: sort and group are not quick filters, per sé - even if they reuse the
QuickFilterimplementation. - DRY up quick filter slots - perhaps with a
build_quick_filter(**kwargs)(similar toActionList#build_item) - fail fast: if you implement 2, you could also consider crashing earlier on - when the slot method is called rather than at
before_render.
def initialize(...)
# ...
@filter_count = 0
end
def build_quick_filter(**kwargs)
raise ArgumentError, "SubHeader supports a maximum of 5 quick_filters" if @filter_count > 5
deny_tag_argument(**kwargs)
kwargs[:tag] = :div
kwargs[:mr] ||= 2
@filter_count += 1
QuickFilter.new(**kwargs)
end6290ad6 to
c30bff1
Compare
c30bff1 to
76f0739
Compare
What are you trying to accomplish?
Introduce specific slots for sort and group quick filters to ensure the order is enforced
List the issues that this change affects.
https://community.openproject.org/wp/OP-19305
Risk Assessment