Added infinite scroll support to filter options#27606
Open
kevinansfield wants to merge 1 commit into
Open
Conversation
Contributor
WalkthroughThis PR introduces infinite-scroll capability for paginated filter and select options across the Ghost admin interface. A new 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
kevinansfield
force-pushed
the
codex/filter-option-source-refactor
branch
from
0db9884 to
0435e09
Compare
kevinansfield
force-pushed
the
codex/filter-options-infinite-scroll
branch
5 times, most recently
from
fb602c2 to
64e034f
Compare
kevinansfield
force-pushed
the
codex/filter-option-source-refactor
branch
from
0435e09 to
9c5cec2
Compare
kevinansfield
force-pushed
the
codex/filter-options-infinite-scroll
branch
from
64e034f to
09fef38
Compare
kevinansfield
force-pushed
the
codex/filter-options-infinite-scroll
branch
from
09fef38 to
3b807c1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/shade/src/components/ui/filters.tsx (1)
1123-1130:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHide the empty state while more pages are available.
CommandEmptystill renders whenever the current page is empty, even ifoptionSource.hasMoreis true. That produces a misleading “No results found” message alongside a Load more control on paginated sources. Please gate this on!optionSource.hasMorehere, and mirror the same check in the combobox renderer.♻️ Proposed fix
- ) : ( - <CommandEmpty>{context.i18n.noResultsFound}</CommandEmpty> - )} + ) : !optionSource.hasMore ? ( + <CommandEmpty>{context.i18n.noResultsFound}</CommandEmpty> + ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/components/ui/filters.tsx` around lines 1123 - 1130, The empty-state rendering shows CommandEmpty even when more pages exist; update the conditional in the list renderer (the block using optionSource.isInitialLoad and CommandEmpty) to only render <CommandEmpty> when optionSource.hasMore is false (i.e., !optionSource.hasMore), and apply the same gating in the combobox renderer where the empty message is displayed so that the “No results found” message is suppressed while optionSource.hasMore is true; locate references to optionSource.isInitialLoad, optionSource.hasMore, CommandEmpty and the combobox renderer to apply the change.apps/shade/src/components/ui/multi-select-combobox.tsx (1)
327-335:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHide the empty state while more pages remain.
This path still renders
CommandEmptywhenever the current page is empty, even ifsource.hasMoreis true. That shows “No results found” alongside a Load more control for paginated sources. Please gate this on!source.hasMorehere too.♻️ Proposed fix
- ) : ( - <CommandEmpty>{i18n.noResultsFound}</CommandEmpty> - )} + ) : !source.hasMore ? ( + <CommandEmpty>{i18n.noResultsFound}</CommandEmpty> + ) : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/shade/src/components/ui/multi-select-combobox.tsx` around lines 327 - 335, The empty-state rendering shows CommandEmpty even when pagination indicates more pages; update the conditional inside CommandList to render CommandEmpty only when not initial load AND source.hasMore is false: check source.isInitialLoad first, otherwise render CommandEmpty only if !source.hasMore, using the existing symbols (source.isInitialLoad, source.hasMore, CommandEmpty, i18n.noResultsFound, CommandList) so the "No results found" message is suppressed while more pages remain.
🧹 Nitpick comments (1)
e2e/tests/admin/members/multiselect-filters.test.ts (1)
170-184: ⚡ Quick winExercise the sentinel path here, not only the fallback button.
This test still clicks the “Load more” button, so it would pass even if the new infinite-scroll wiring stopped requesting page 2 automatically. Please trigger the scroll sentinel and assert the next page arrives without a manual click.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/tests/admin/members/multiselect-filters.test.ts` around lines 170 - 184, Replace the manual "Load more" click with exercising the infinite-scroll sentinel: after membersPage.openFilterValue('Label'), locate the scroll sentinel element (e.g. the element used for infinite scroll — look for a test id or class used by the filter value list, such as a data-testid like "infinite-scroll-sentinel" or the last list item) and call scrollIntoView/scrollIntoViewIfNeeded on it so the next page is requested automatically; then assert requestedLabelPages.has(2) and that membersPage.getFilterOption(/Infinite Label 105/) is visible. Keep references to mockPaginatedLabels, seedMembersAndNavigate, membersPage.openFilterField, membersPage.getFilterOption and membersPage.openFilterValue to find the right place to modify the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/shade/src/components/ui/filters.tsx`:
- Around line 1123-1130: The empty-state rendering shows CommandEmpty even when
more pages exist; update the conditional in the list renderer (the block using
optionSource.isInitialLoad and CommandEmpty) to only render <CommandEmpty> when
optionSource.hasMore is false (i.e., !optionSource.hasMore), and apply the same
gating in the combobox renderer where the empty message is displayed so that the
“No results found” message is suppressed while optionSource.hasMore is true;
locate references to optionSource.isInitialLoad, optionSource.hasMore,
CommandEmpty and the combobox renderer to apply the change.
In `@apps/shade/src/components/ui/multi-select-combobox.tsx`:
- Around line 327-335: The empty-state rendering shows CommandEmpty even when
pagination indicates more pages; update the conditional inside CommandList to
render CommandEmpty only when not initial load AND source.hasMore is false:
check source.isInitialLoad first, otherwise render CommandEmpty only if
!source.hasMore, using the existing symbols (source.isInitialLoad,
source.hasMore, CommandEmpty, i18n.noResultsFound, CommandList) so the "No
results found" message is suppressed while more pages remain.
---
Nitpick comments:
In `@e2e/tests/admin/members/multiselect-filters.test.ts`:
- Around line 170-184: Replace the manual "Load more" click with exercising the
infinite-scroll sentinel: after membersPage.openFilterValue('Label'), locate the
scroll sentinel element (e.g. the element used for infinite scroll — look for a
test id or class used by the filter value list, such as a data-testid like
"infinite-scroll-sentinel" or the last list item) and call
scrollIntoView/scrollIntoViewIfNeeded on it so the next page is requested
automatically; then assert requestedLabelPages.has(2) and that
membersPage.getFilterOption(/Infinite Label 105/) is visible. Keep references to
mockPaginatedLabels, seedMembersAndNavigate, membersPage.openFilterField,
membersPage.getFilterOption and membersPage.openFilterValue to find the right
place to modify the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02ef933e-1b81-4edc-be7b-d4265e8cee83
📒 Files selected for processing (10)
apps/posts/src/components/label-picker/label-picker.tsxapps/posts/src/hooks/filter-sources/create-combined-value-source.tsapps/posts/test/unit/hooks/create-combined-value-source.test.tsxapps/shade/src/components.tsapps/shade/src/components/ui/filters.tsxapps/shade/src/components/ui/multi-select-combobox.tsxapps/shade/src/components/ui/use-filter-options-infinite-scroll.tsapps/shade/test/unit/components/ui/filters.test.tsxe2e/helpers/pages/admin/members/members-list-page.tse2e/tests/admin/members/multiselect-filters.test.ts
no issue Large filter option lists now share one guarded load-more trigger, while retaining the manual fallback button and covering remote label pagination in E2E.
kevinansfield
force-pushed
the
codex/filter-options-infinite-scroll
branch
from
3b807c1 to
ede93a7
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/shade/src/components/patterns/filters.tsx (1)
1332-1339:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSuppress the empty state while more pages are available.
Line 1332 still renders
CommandEmptywhenever the initial load is done, so this list can show “No results found” at the same time as the load-more control. That is the same contradictory state the label picker change avoids.Suggested fix
- {optionSource.isInitialLoad ? ( + {optionSource.isInitialLoad ? ( <div className="flex items-center justify-center py-6 text-sm text-muted-foreground"> <Loader2 className="mr-2 size-4 animate-spin" /> {context.i18n.loading} </div> - ) : ( + ) : !optionSource.hasMore ? ( <CommandEmpty>{context.i18n.noResultsFound}</CommandEmpty> - )} + ) : null}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/shade/src/components/patterns/filters.tsx` around lines 1332 - 1339, The empty-state is shown whenever optionSource.isInitialLoad is false, which leads to a "No results found" message even if more pages can be loaded; update the conditional that renders CommandEmpty so it only shows when loading is finished AND there are no additional pages (e.g., check optionSource.hasMore / optionSource.canLoadMore / optionSource.hasNextPage or similar boolean used by your load-more control). Replace the current else-branch condition to require !optionSource.isInitialLoad && !optionSource.<has-more-flag> before rendering CommandEmpty (keep the initial load branch with Loader2 unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/shade/src/components/patterns/filters.tsx`:
- Around line 1332-1339: The empty-state is shown whenever
optionSource.isInitialLoad is false, which leads to a "No results found" message
even if more pages can be loaded; update the conditional that renders
CommandEmpty so it only shows when loading is finished AND there are no
additional pages (e.g., check optionSource.hasMore / optionSource.canLoadMore /
optionSource.hasNextPage or similar boolean used by your load-more control).
Replace the current else-branch condition to require !optionSource.isInitialLoad
&& !optionSource.<has-more-flag> before rendering CommandEmpty (keep the initial
load branch with Loader2 unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b26f19a-0f25-4ff3-9374-901793ab2713
📒 Files selected for processing (6)
apps/posts/src/components/label-picker/label-picker.tsxapps/posts/src/hooks/filter-sources/create-combined-value-source.tsapps/posts/test/unit/hooks/create-combined-value-source.test.tsxapps/shade/src/components.tsapps/shade/src/components/patterns/filters.tsxapps/shade/src/components/ui/multi-select-combobox.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/posts/src/hooks/filter-sources/create-combined-value-source.ts
- apps/shade/src/components.ts
- apps/shade/src/components/ui/multi-select-combobox.tsx
- apps/posts/test/unit/hooks/create-combined-value-source.test.tsx
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
1 participant
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.
ref https://linear.app/ghost/issue/BER-3334/
Summary
Stack
Stacked on #27605.