Added missing empty state for members#27463
Merged
jonatansberg merged 5 commits intomainfrom Apr 29, 2026
Merged
Conversation
No ref Adds back a (Shade-driven) empty state for Members to emulate the existing one in Ember.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughA new MembersEmptyState component was added and integrated into MembersPage, with a membershipsEnabled prop sourced from settings. MembersHelpCards were added and are shown conditionally. EmptyIndicator’s actions container spacing was reduced. A NewMember type and useAddMember mutation were introduced to POST members. E2E updates: empty-state text changed to "Start building your audience", an addYourselfButton locator was added, and a test was added that clicks the “add yourself” control and verifies a single-member row appears. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/posts/src/views/members/components/members-empty-state.tsx`:
- Around line 11-18: The code treats memberships as enabled when settings are
unresolved because membersSignupAccess becomes undefined; update the
membershipsEnabled calculation to only evaluate after settings are loaded (e.g.,
check settingsData or settingsData.settings exists) — for example in the
component that calls useBrowseSettings, change the membershipsEnabled derivation
(currently using membersSignupAccess and membershipsEnabled) to first gate on
settingsData (or a loading flag) and only set membershipsEnabled =
getSettingValue<string>(settingsData.settings, 'members_signup_access') !==
'none' when settingsData is present so the CTA is not shown while settings are
still loading.
- Around line 27-39: The POST to `${apiRoot}/members/` in this component is
missing the framework's required fetch settings (e.g., credentials and
cache/version headers); update the call to include credentials: 'include' and
add headers like 'app-pragma': 'no-cache' and the app/version header matching
the pattern in import-members-modal.tsx, or better yet, extract this logic into
a new mutation hook (e.g., useCreateMembers) mirroring existing hooks such as
useAddTier/useAddNewsletter so API auth/headers are centralized and reusable;
modify the code that calls getGhostPaths() and currentUser to use that hook or
the corrected fetch options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5bcc814-af29-4a4b-b70b-6a98c1d556e3
📒 Files selected for processing (3)
apps/posts/src/views/members/components/members-empty-state.tsxapps/posts/src/views/members/members.tsxapps/shade/src/components/ui/empty-indicator.tsx
Comment on lines
+11
to
+18
| const {data: settingsData} = useBrowseSettings({}); | ||
| const {data: currentUser} = useCurrentUser(); | ||
| const navigate = useNavigate(); | ||
| const [isAdding, setIsAdding] = useState(false); | ||
|
|
||
| const {assetRoot} = getGhostPaths(); | ||
| const membersSignupAccess = getSettingValue<string>(settingsData?.settings ?? null, 'members_signup_access'); | ||
| const membershipsEnabled = membersSignupAccess !== 'none'; |
Contributor
There was a problem hiding this comment.
Don’t treat unresolved settings as memberships enabled.
While useBrowseSettings is still loading, membersSignupAccess is undefined, so undefined !== 'none' renders the enabled CTA even on sites where memberships are disabled. Gate the branch until settings are loaded.
Proposed fix
-import {Button, EmptyIndicator} from '@tryghost/shade/components';
+import {Button, EmptyIndicator, LoadingIndicator} from '@tryghost/shade/components';
@@
- const {data: settingsData} = useBrowseSettings({});
+ const {data: settingsData, isLoading: isSettingsLoading} = useBrowseSettings({});
@@
const {assetRoot} = getGhostPaths();
const membersSignupAccess = getSettingValue<string>(settingsData?.settings ?? null, 'members_signup_access');
const membershipsEnabled = membersSignupAccess !== 'none';
+
+ if (isSettingsLoading || !settingsData?.settings) {
+ return (
+ <div className="flex h-full items-center justify-center px-4">
+ <LoadingIndicator size="lg" />
+ </div>
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const {data: settingsData} = useBrowseSettings({}); | |
| const {data: currentUser} = useCurrentUser(); | |
| const navigate = useNavigate(); | |
| const [isAdding, setIsAdding] = useState(false); | |
| const {assetRoot} = getGhostPaths(); | |
| const membersSignupAccess = getSettingValue<string>(settingsData?.settings ?? null, 'members_signup_access'); | |
| const membershipsEnabled = membersSignupAccess !== 'none'; | |
| const {data: settingsData, isLoading: isSettingsLoading} = useBrowseSettings({}); | |
| const {data: currentUser} = useCurrentUser(); | |
| const navigate = useNavigate(); | |
| const [isAdding, setIsAdding] = useState(false); | |
| const {assetRoot} = getGhostPaths(); | |
| const membersSignupAccess = getSettingValue<string>(settingsData?.settings ?? null, 'members_signup_access'); | |
| const membershipsEnabled = membersSignupAccess !== 'none'; | |
| if (isSettingsLoading || !settingsData?.settings) { | |
| return ( | |
| <div className="flex h-full items-center justify-center px-4"> | |
| <LoadingIndicator size="lg" /> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/posts/src/views/members/components/members-empty-state.tsx` around lines
11 - 18, The code treats memberships as enabled when settings are unresolved
because membersSignupAccess becomes undefined; update the membershipsEnabled
calculation to only evaluate after settings are loaded (e.g., check settingsData
or settingsData.settings exists) — for example in the component that calls
useBrowseSettings, change the membershipsEnabled derivation (currently using
membersSignupAccess and membershipsEnabled) to first gate on settingsData (or a
loading flag) and only set membershipsEnabled =
getSettingValue<string>(settingsData.settings, 'members_signup_access') !==
'none' when settingsData is present so the CTA is not shown while settings are
still loading.
Contributor
E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 24662282217 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
No ref Aligned the React members empty state with the framework API path so the CTA waits for resolved settings, invalidates member queries, and keeps the help cards consistent with the Ember behavior.
Member
jonatansberg
left a comment
jonatansberg
left a comment
There was a problem hiding this comment.
I pushed a commit that moved the network call into the framework package, added an E2E test and cleaned up a few other things.
If the change to the Shade empty state component was intentional, I think we can go ahead and merge this.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/posts/src/views/members/members.tsx`:
- Line 80: The empty-state help cards are shown even when memberships are
disabled because shouldShowMembersHelpCards does not check membershipsEnabled;
update the boolean expression used to compute shouldShowMembersHelpCards (and
the identical check at the other occurrence) to also require membershipsEnabled
=== true so the help cards are only rendered when memberships are enabled,
ensuring the disabled-memberships flow never shows the “building your audience”
cards.
- Line 211: The onMemberCreated handler currently calls void refetch(),
discarding the returned promise; update the invocation in the MembersEmptyState
props so it returns the refetch promise (i.e., use return refetch() or simply
refetch()) instead of void refetch() to allow callers to await completion;
locate the MembersEmptyState usage in members.tsx and change the onMemberCreated
prop accordingly.
In `@e2e/tests/admin/members/list.test.ts`:
- Line 41: Rename the test description string for the test currently declared as
test('adds the current user as a member from the empty state', ...) to follow
the E2E naming format 'what is tested - expected outcome' in lowercase; update
the test declaration in e2e/tests/admin/members/list.test.ts (the test(...)
call) to a name like 'add current user as member - current user appears in
members list' or another equivalent lowercase "{what} - {expected outcome}"
phrasing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36f9893b-4a7e-4bd6-bab2-eecd823d64f4
📒 Files selected for processing (6)
apps/admin-x-framework/src/api/members.tsapps/posts/src/views/members/components/members-empty-state.tsxapps/posts/src/views/members/components/members-help-cards.tsxapps/posts/src/views/members/members.tsxe2e/helpers/pages/admin/members/members-list-page.tse2e/tests/admin/members/list.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- e2e/helpers/pages/admin/members/members-list-page.ts
- apps/posts/src/views/members/components/members-empty-state.tsx
| const hasFilters = filters.length > 0; | ||
| const shouldShowMobileSearchRow = showMobileSearch; | ||
| const shouldShowFiltersRow = hasFilters; | ||
| const shouldShowMembersHelpCards = !hasFilterOrSearch && !shouldShowLoading && !isError && totalMembers < MEMBERS_HELP_CARDS_LIMIT; |
Contributor
There was a problem hiding this comment.
Hide help cards when memberships are disabled.
shouldShowMembersHelpCards ignores membershipsEnabled, so the disabled-memberships empty state can still be followed by “building your audience” cards, which conflicts with the disabled-state flow.
Proposed fix
- const shouldShowMembersHelpCards = !hasFilterOrSearch && !shouldShowLoading && !isError && totalMembers < MEMBERS_HELP_CARDS_LIMIT;
+ const shouldShowMembersHelpCards = membershipsEnabled && !hasFilterOrSearch && !shouldShowLoading && !isError && totalMembers < MEMBERS_HELP_CARDS_LIMIT;Also applies to: 228-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/posts/src/views/members/members.tsx` at line 80, The empty-state help
cards are shown even when memberships are disabled because
shouldShowMembersHelpCards does not check membershipsEnabled; update the boolean
expression used to compute shouldShowMembersHelpCards (and the identical check
at the other occurrence) to also require membershipsEnabled === true so the help
cards are only rendered when memberships are enabled, ensuring the
disabled-memberships flow never shows the “building your audience” cards.
Returned the refresh promise and renamed the E2E scenario to match the requested naming format.
|
The empty state callback should not expose React Query's refetch result because the component only needs to wait for the refresh side effect.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/posts/src/views/members/members.tsx (1)
80-80:⚠️ Potential issue | 🟡 MinorGate help cards with
membershipsEnabledto avoid conflicting disabled-state UI.Line 80 can still evaluate true when memberships are disabled, so Line 230 may render audience-building cards under the disabled-memberships empty state.
Proposed fix
- const shouldShowMembersHelpCards = !hasFilterOrSearch && !shouldShowLoading && !isError && totalMembers < MEMBERS_HELP_CARDS_LIMIT; + const shouldShowMembersHelpCards = membershipsEnabled && !hasFilterOrSearch && !shouldShowLoading && !isError && totalMembers < MEMBERS_HELP_CARDS_LIMIT;Also applies to: 230-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/posts/src/views/members/members.tsx` at line 80, The help-card gating condition (shouldShowMembersHelpCards) can be true even when memberships are disabled, causing audience-building cards to appear under the disabled-memberships empty state; update the boolean expression that defines shouldShowMembersHelpCards to also require membershipsEnabled (i.e., include membershipsEnabled && in the condition that currently uses !hasFilterOrSearch && !shouldShowLoading && !isError && totalMembers < MEMBERS_HELP_CARDS_LIMIT) so the audience-building cards are suppressed when membershipsEnabled is false and the render path that shows those cards (around the audience-building cards at the render site referenced in the diff) respects the disabled-memberships state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/posts/src/views/members/members.tsx`:
- Line 80: The help-card gating condition (shouldShowMembersHelpCards) can be
true even when memberships are disabled, causing audience-building cards to
appear under the disabled-memberships empty state; update the boolean expression
that defines shouldShowMembersHelpCards to also require membershipsEnabled
(i.e., include membershipsEnabled && in the condition that currently uses
!hasFilterOrSearch && !shouldShowLoading && !isError && totalMembers <
MEMBERS_HELP_CARDS_LIMIT) so the audience-building cards are suppressed when
membershipsEnabled is false and the render path that shows those cards (around
the audience-building cards at the render site referenced in the diff) respects
the disabled-memberships state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d3611462-27ed-4cee-8b93-bae9a362fb81
📒 Files selected for processing (1)
apps/posts/src/views/members/members.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
2 participants
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.
No ref
Adds back a (Shade-driven) empty state for Members to emulate the existing one in Ember.
.