fix: Show All button in DirectoryGroup now properly expands sessions#161
Open
edspencer wants to merge 2 commits into
Open
fix: Show All button in DirectoryGroup now properly expands sessions#161edspencer wants to merge 2 commits into
edspencer wants to merge 2 commits into
Conversation
Fixes #150 - Add showAll state to track whether user clicked "Show all" - When showAll is true, render all filteredSessions instead of slicing to INITIAL_SESSIONS_SHOWN - Button now shows all locally-loaded sessions first, then fetches more from server if needed - Hide button after it's been clicked (when showAll is true) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds a changeset file documenting a patch release and modifies DirectoryGroup component to fix the non-functional "Show All" button. Introduces local Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Deploying herdctl with
|
| Latest commit: |
dfc37c8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3bd151bf.herdctl.pages.dev |
| Branch Preview URL: | https://fix-150-show-all-button.herdctl.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/web/src/client/src/components/all-chats/DirectoryGroup.tsx (1)
65-65: Add an explicit boolean type forshowAllstate.Line 65 currently relies on inference; prefer explicit typing for consistency with strict TS conventions.
Suggested diff
- const [showAll, setShowAll] = useState(false); + const [showAll, setShowAll] = useState<boolean>(false);As per coding guidelines,
**/*.{ts,tsx}: Use strict TypeScript with explicit types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/client/src/components/all-chats/DirectoryGroup.tsx` at line 65, The state declaration for showAll uses type inference; update it to an explicit boolean type to satisfy strict TS rules by changing the useState call that defines showAll and setShowAll (the useState import or invocation in DirectoryGroup.tsx) to declare useState<boolean>(...) so showAll is explicitly typed as boolean while preserving the initial value and existing usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/client/src/components/all-chats/DirectoryGroup.tsx`:
- Around line 80-85: handleShowAll currently calls loadMoreGroupSessions once
(which fetches limit:50 in all-chats-slice.ts) then hides the "Load more" button
via setShowAll(true), so groups with >50 remaining sessions become inaccessible;
update handleShowAll in DirectoryGroup.tsx to either (a) loop/await
loadMoreGroupSessions(group.encodedPath) until hasMoreOnServer is false so all
pages are fetched before hiding the button, or (b) keep the load-more control
visible (do not call setShowAll(true) or hide the button) and let the user
repeatedly invoke loadMoreGroupSessions while hasMoreOnServer remains true;
reference the symbols handleShowAll, loadMoreGroupSessions, and hasMoreOnServer
when making the change.
---
Nitpick comments:
In `@packages/web/src/client/src/components/all-chats/DirectoryGroup.tsx`:
- Line 65: The state declaration for showAll uses type inference; update it to
an explicit boolean type to satisfy strict TS rules by changing the useState
call that defines showAll and setShowAll (the useState import or invocation in
DirectoryGroup.tsx) to declare useState<boolean>(...) so showAll is explicitly
typed as boolean while preserving the initial value and existing usage.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/fix-show-all-button.mdpackages/web/src/client/src/components/all-chats/DirectoryGroup.tsx
Comment on lines
+80
to
+85
| // First, show all locally-loaded sessions | ||
| setShowAll(true); | ||
| // Then fetch more from server if there are any | ||
| if (hasMoreOnServer) { | ||
| loadMoreGroupSessions(group.encodedPath); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Searching for loadMoreGroupSessions definition and usage =="
rg -nP --type=ts --type=tsx -C4 '\bloadMoreGroupSessions\b'
echo
echo "== Checking DirectoryGroup.tsx content around lines 80-85 and 154 =="
fd -e tsx -e ts "DirectoryGroup" | head -5Repository: edspencer/herdctl
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Finding DirectoryGroup.tsx =="
fd "DirectoryGroup" -e tsx -e ts
echo
echo "== Searching for loadMoreGroupSessions without type restriction =="
rg -n 'loadMoreGroupSessions' --max-count=20
echo
echo "== Checking pagination-related patterns =="
rg -n 'hasMore|nextCursor|offset|limit|page' | head -30Repository: edspencer/herdctl
Length of output: 2340
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Reading all-chats-slice.ts loadMoreGroupSessions implementation =="
cat -n packages/web/src/client/src/store/all-chats-slice.ts | sed -n '100,150p'
echo
echo "== Reading DirectoryGroup.tsx around lines 80-85 and 154 =="
cat -n packages/web/src/client/src/components/all-chats/DirectoryGroup.tsx | sed -n '60,100p'
echo
echo "== Reading DirectoryGroup.tsx around line 154 =="
cat -n packages/web/src/client/src/components/all-chats/DirectoryGroup.tsx | sed -n '145,165p'Repository: edspencer/herdctl
Length of output: 4362
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Checking for any pagination continuation or hasMore tracking =="
grep -n "sessionCount\|sessions.length\|hasMore" packages/web/src/client/src/store/all-chats-slice.ts | head -20
echo
echo "== Check if there's re-fetching logic elsewhere =="
rg -n "loadMoreGroupSessions" packages/web/src/client/src/store/ -A5 -B5
echo
echo "== Check API response structure =="
rg -n "fetchDirectoryGroupSessions" packages/web/src/client/src/ -A10 | head -40Repository: edspencer/herdctl
Length of output: 4916
Confirm pagination handles groups with >50 additional sessions.
loadMoreGroupSessions fetches with limit: 50 (all-chats-slice.ts:118), but handleShowAll (DirectoryGroup.tsx:79–85) calls it only once. If a group has more than 50 additional sessions beyond those already loaded, the button disappears after the first fetch (line 154), leaving remaining sessions inaccessible. Either loop until hasMoreOnServer is false, or preserve a visible "Load more" button when additional server data exists.
Also applies to: 154-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/client/src/components/all-chats/DirectoryGroup.tsx` around
lines 80 - 85, handleShowAll currently calls loadMoreGroupSessions once (which
fetches limit:50 in all-chats-slice.ts) then hides the "Load more" button via
setShowAll(true), so groups with >50 remaining sessions become inaccessible;
update handleShowAll in DirectoryGroup.tsx to either (a) loop/await
loadMoreGroupSessions(group.encodedPath) until hasMoreOnServer is false so all
pages are fetched before hiding the button, or (b) keep the load-more control
visible (do not call setShowAll(true) or hide the button) and let the user
repeatedly invoke loadMoreGroupSessions while hasMoreOnServer remains true;
reference the symbols handleShowAll, loadMoreGroupSessions, and hasMoreOnServer
when making the change.
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.
Summary
Fixes #150
showAllstate to track whether user clicked "Show all" buttonshowAllis true, component renders allfilteredSessionsinstead of slicing toINITIAL_SESSIONS_SHOWNhasMoreOnServeris trueshowAllis true)Changes
useStatefrom ReactshowAllstate (defaults tofalse)filteredSessionsbased onshowAllstatehandleShowAllto set state and conditionally fetch from servershowAllis trueTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes