Skip to content

feat(governance): instant vote reorder, snappier dropdowns, cleaner focus outline - #1719

Merged
jwalkingjew merged 10 commits into
masterfrom
governance-ux-polish
Apr 28, 2026
Merged

jwalkingjew merged 10 commits into
masterfrom
governance-ux-polish

Conversation

@jwalkingjew

@jwalkingjew jwalkingjew commented Apr 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Three governance UX improvements split out from #1692 so they can ship now; the red-dot + notification-service integration stays on the original PR pending webhook registration with the gaia team.

  • Optimistic vote card reorder. Clicking Accept/Reject on a proposal sinks its card to the bottom of its bucket (executable / active) immediately, via a jotai-driven CSS order bump. Rolled back if the tx errors. Fixes the multi-second gap between You accepted appearing and the list finally reordering.
  • Governance home dropdowns. Selecting a space / category / status now updates the trigger label instantly via local pendingLabel state, cleared once the server-derived label catches up. Removed the Suspense key that forced a skeleton flash on every filter change — the remaining Suspense fallback now fires only when the component actually suspends (cached / prefetched responses swap without any skeleton).
  • Button focus outline. Switched Button primitive's focus: styles to focus-visible: so mouse clicks no longer leave a lingering bold inset border. Keyboard focus still shows it.
  • Infinite-scroll loading state. On /home, the "Load more…" text-only button during pagination is replaced by two card-shaped skeletons (matching the proposal card spacing) so the loading state reads the same as the rest of the page. Also wrapped appended pages in keyed React.Fragments so appends don't thrash the whole subtree.
  • Extracted LoadingSkeleton into its own module so the client infinite-scroll component can consume it without dragging server-only transitive imports (@opentelemetry/context-async-hooks → node:async_hooks) into the client bundle.

Known pre-existing issue NOT fixed here

On /home, clicking "Load more" (or scrolling to trigger the IntersectionObserver) still scrolls the viewport back to the top of the proposal list. This behavior is on master as well and was not introduced by this PR. Root cause is architectural: loadMoreHomeProposalsAction is a Next.js server action, and every server-action call ships a fresh RSC tree for the current route. Committing that tree re-reconciles every proposal card above the viewport and breaks the browser's scroll anchor.

Patching around it from inside the client component (rAF restore, scroll pinning) fights the RSC commit visibly. The proper fix is to refactor the pagination server action into a route handler that returns serializable proposal data and render the cards client-side — deferring that to its own ticket since it touches the membership / content proposal card server components, profile / space resolution, and cachedFetchSpace usage.

What's not in this PR

  • Sidebar red-dot behavior (hasPendingVotes → pendingVoteProposalIds, parallelized scan, confirm-vote wiring).
  • Webhook receiver + Redis-backed pending-votes cache.
  • Sidebar auto-refresh / polling changes.

Those remain on #1692, which is blocked on gaia registering the webhook endpoint. Once this PR merges, that branch will rebase cleanly to contain only the red-dot / notification-service work.

Test plan

  • On a governance space page, open a pending proposal and click Accept or Reject. The card should visibly sink to the bottom of its section before the tx round-trip resolves.
  • If the tx errors, the card should bounce back to its original position.
  • On /home, change the space / category / status dropdown. The trigger button should show the new label immediately on click.
  • On cached / prefetched dropdown navigation, no skeleton should flash.
  • Mouse-click any variant="secondary" button (e.g. a dropdown trigger). No persistent thick dark outline should remain after the click.
  • Keyboard-focus (Tab) the same button. The focus outline should still appear.
  • On /home with enough proposals to paginate, scroll to the bottom; two card-shaped skeletons appear during the fetch (no "Loading…" text); the freshly appended page is spaced the same as the earlier cards.

…ocus outline

Three UX improvements to the governance experience:

- Optimistic vote card reorder. Clicking Accept/Reject on a proposal
  now sinks the card to the bottom of its bucket (executable/active)
  immediately via a jotai-driven CSS order bump, rolled back if the
  tx errors. No more multi-second gap between 'You accepted' and
  the reorder.

- Governance home dropdown polish. The space/category/status menus
  on /home now show the chosen label immediately on click (local
  pendingLabel state), cleared once the server-derived label catches
  up. Removed the Suspense key that forced a full skeleton flash on
  every filter change — the remaining Suspense fallback now only
  fires when the component actually suspends. The dropdown buttons
  also lose their lingering bold focus outline on mouse click by
  switching focus: styles to focus-visible: in the Button design
  system primitive.

Left deliberately untouched: sidebar red-dot behavior and the
notification-service integration — those ship separately once the
webhook registration is coordinated with the gaia team.
@vercel

vercel Bot commented Apr 24, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
geogenesis Ready Ready Preview Apr 28, 2026 6:23pm

Request Review

Two issues on the governance home infinite-scroll:

- The 'Load more' button showed a plain 'Loading...' text while the
  next page fetched, inconsistent with the rest of the page which
  uses the card-shaped LoadingSkeleton. Swap to the skeleton while
  loading; the sentinel moves from the button itself to a wrapper
  div so the IntersectionObserver target stays mounted across the
  loading transition.

- Appending a new page sometimes scrolled the viewport back to the
  top. Cause: loadMoreNodes was rendered as a raw React.ReactNode[]
  with no keys, so React reconciled the whole list on every append
  and the scroll anchor above the viewport could be lost. Wrap each
  node in a keyed React.Fragment, matching the space-level governance
  infinite scroll which never had the issue.
The client infinite-scroll component imported LoadingSkeleton from
component.tsx (a server component). Turbopack traced the import and
tried to bundle component.tsx's server-only transitive deps
(@opentelemetry/context-async-hooks pulls node:async_hooks) into the
client bundle, breaking the build with 'Module not found: async_hooks'.

Move LoadingSkeleton into its own module with no server-only imports
so either side can consume it.
- Render two LoadingSkeleton cards inside the sentinel and apply
  space-y-2 so the gap matches the actual proposal-card spacing
  produced by PendingProposalsPage.

- Capture window.scrollY before calling loadMoreHomeProposalsAction
  and restore it across two rAFs after the new page mounts. The
  Next.js server action call triggers a server-component re-render
  for the current route, which reshuffles the proposal-list
  reconciliation enough to break the browser's scroll anchor and
  snap the viewport back to the top of the list. The double rAF
  catches both the React commit and any late image-driven reflow.
…e infinite scroll

Two problems left after the previous attempt:

- The single-tick rAF restore wasn't enough to fight the scroll jump.
  The Next.js server-action RSC refetch commits asynchronously and
  images on the freshly mounted proposal cards can reflow several
  frames later, each one re-snapping the viewport up. Replace the
  one-shot rAF with a 16ms interval that pins window.scrollY back to
  the captured position for up to 800ms after the action fires, then
  releases. Aborted loads also stop the pin immediately.

- The first skeleton (and the first appended page when not loading)
  rendered flush against the last card of page 0 because the wrapper
  had space-y-2 but no top margin of its own. Add mt-2 so the gap to
  the cards above matches the gap between cards.
The interval-based restore was fighting the RSC reflow instead of
resolving it, producing a visible scroll glitch before the viewport
still ended up snapped to the top of the list. Back out the pin-and-
restore logic; leave the two-skeleton-card visual and the mt-2 top
gap alone since both are unrelated wins. Scroll-to-top during
infinite-scroll remains unresolved and will need a different
approach (likely refactoring the server action into a route handler
so there's no RSC refetch to reconcile).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR ships several governance UX improvements on /home and space governance pages, including optimistic vote-driven list reordering, instant dropdown trigger label updates, cleaner focus styling, and improved infinite-scroll loading visuals.

Changes:

  • Add optimistic “voted” tracking (Jotai) and apply it to proposal list ordering via CSS order.
  • Make governance filter dropdown triggers update labels immediately without forcing Suspense remounts.
  • Improve pagination/loading UI on /home by replacing the “Load more…” text state with proposal-card skeletons and extracting LoadingSkeleton into a shared module.
  • Update Button focus styles from focus: to focus-visible: for better mouse/keyboard behavior.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
apps/web/partials/governance/proposal-list-item.tsx New client wrapper applying CSS order based on optimistic vote state
apps/web/partials/governance/optimistic-voted-atom.ts New Jotai atom + hooks to track optimistic voted proposal IDs
apps/web/partials/governance/governance-proposals-list.tsx Bucketing + base ordering for proposals; wraps items in ProposalListItem; adjusts open-proposal sorting
apps/web/partials/active-proposal/accept-or-reject.tsx Adds optimistic vote + refresh/error callbacks to vote mutations
apps/web/design-system/button.tsx Switches focus styling to focus-visible across button variants
apps/web/app/home/personal-home-dashboard.tsx Adds pendingLabel for instant dropdown trigger updates
apps/web/app/home/loading-skeleton.tsx Extracts reusable LoadingSkeleton component
apps/web/app/home/home-proposals-infinite-scroll.tsx Replaces “Loading…” button state with skeleton cards; wraps appended pages in keyed fragments
apps/web/app/home/component.tsx Removes Suspense key and imports extracted LoadingSkeleton
apps/web/app/home/accept-or-reject-member.tsx Adds optimistic vote + refresh/error callbacks to vote mutations
apps/web/app/home/accept-or-reject-editor.tsx Adds optimistic vote + refresh/error callbacks to vote mutations
Comments suppressed due to low confidence (1)

apps/web/app/home/component.tsx:66

  • Removing the Suspense key can preserve stateful client components across filter changes. In particular, HomeProposalsInfiniteScroll keeps loadMoreNodes/currentPageRef in state/refs and does not reset when governanceFilters changes, so switching filters can append pages from the previous filter into the new list. Consider adding a stable key (derived from governanceFilters/proposalType/viewer) to HomeProposalsInfiniteScroll, or resetting its internal state in an effect when those props change, so each filter combination starts from a clean pagination state.
              <React.Suspense
                fallback={
                  <div className="space-y-2">
                    <LoadingSkeleton />
                    <LoadingSkeleton />
                  </div>
                }
              >

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +62 to +75
const onVoteError = () => {
removeOptimisticVote(proposalId);
};

const onApprove = () => {
setHasApproved(true);
vote('ACCEPT');
addOptimisticVote(proposalId);
vote('ACCEPT', { onSuccess: onVoteSuccess, onError: onVoteError });
};

const onReject = () => {
setHasRejected(true);
vote('REJECT');
addOptimisticVote(proposalId);
vote('REJECT', { onSuccess: onVoteSuccess, onError: onVoteError });

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On vote error you remove the optimistic reorder, but this component has no error UI and the buttons remain disabled because they use disabled={voteStatus !== 'idle'}. If the tx fails, the user can’t retry without a full page refresh. To support the described rollback-on-error behavior, consider resetting local hasApproved/hasRejected on error and/or disabling only while voteStatus === 'pending', or adding an explicit retry state like the governance home cards.

Copilot uses AI. Check for mistakes.
Comment on lines +53 to +70
const onVoteSuccess = () => {
router.refresh();
};

const onVoteError = () => {
removeOptimisticVote(proposalId);
};

const onApprove = () => {
setHasApproved(true);
vote('ACCEPT');
addOptimisticVote(proposalId);
vote('ACCEPT', { onSuccess: onVoteSuccess, onError: onVoteError });
};

const onReject = () => {
setHasRejected(true);
vote('REJECT');
addOptimisticVote(proposalId);
vote('REJECT', { onSuccess: onVoteSuccess, onError: onVoteError });

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onVoteError removes the optimistic reorder but there is no error/retry state in this component and the vote buttons stay disabled via disabled={voteStatus !== 'idle'}. After a failed tx, the user can’t retry without navigating away. Consider resetting local vote-selection state on error and disabling only during pending, or adding a retry UI similar to AcceptOrRejectMember.

Copilot uses AI. Check for mistakes.
href={item.href}
onClick={() => setOpen(false)}
onClick={() => {
if (item.label !== label) setPendingLabel(item.label);

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pending-label logic compares item.label to the server-derived label. If pendingLabel is currently set (because the server label hasn’t caught up yet) and the user selects an item whose label equals the old label, this branch won’t update pendingLabel, leaving the trigger showing the wrong value during navigation. Compare against displayLabel (or always set pendingLabel on selection) to ensure the trigger always reflects the most recently chosen item immediately.

Suggested change
if (item.label !== label) setPendingLabel(item.label);
if (item.label !== displayLabel) setPendingLabel(item.label);

Copilot uses AI. Check for mistakes.
Comment on lines +125 to +151
@@ -120,9 +140,16 @@ export async function GovernanceProposalsList({
const showReopenMenu =
p.status === 'REJECTED' && p.type === 'ADD_EDIT' && getIsProposalEnded(p.status, p.endTime);

const baseOrder = BUCKET_BASE_ORDER[p.bucket] + bucketPositions[p.bucket]++;

return (
<Link
<ProposalListItem
key={p.id}
proposalId={p.id}
baseOrder={baseOrder}
canSink={p.bucket !== 'completed'}
>

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using CSS order to visually reorder list items means DOM order (and therefore screen-reader reading order + keyboard tab order) won’t match what the user sees. It also interacts poorly with the parent divide-y styling, which applies borders based on DOM order—if the first DOM item is “sunk”, the new visually-first item will still have a divider border. Consider reordering in the DOM (e.g., client-side sort based on the optimistic state) and/or switching away from divide-y to per-item borders that don’t depend on sibling order.

Copilot uses AI. Check for mistakes.
Comment on lines +86 to +92
const onVoteSuccess = () => {
router.refresh();
};

const onVoteError = () => {
removeOptimisticVote(proposalId);
};

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onVoteSuccess/onVoteError (and the optimistic sink via addOptimisticVote) are only wired for the initial vote calls. In the existing Retry UI (later in this component), vote(selectedVote) is invoked without these callbacks or re-adding the optimistic vote, so retries won’t refresh the server state and won’t re-sink the card. Consider centralizing the vote invocation so both initial vote and retry share the same optimistic + refresh behavior.

Copilot uses AI. Check for mistakes.
Four small correctness fixes flagged in PR review:

- personal-home-dashboard: dropdown pendingLabel comparison now uses
  displayLabel instead of the server-derived label. Without this, a
  fast follow-up click on the currently-displayed item (whose label
  matches the still-stale label prop) would leave the trigger
  showing the previously chosen label until the server caught up.

- accept-or-reject + accept-or-reject-editor: vote buttons disabled
  on voteStatus === 'pending' instead of !== 'idle'. Previously a
  failed transaction left both buttons permanently disabled with no
  retry path; the optimistic rollback already bounces the card back,
  so the buttons just need to re-enable on error.

- accept-or-reject-member: centralize the vote call into a castVote
  helper so the Retry button goes through the same optimistic-add +
  onSuccess/onError callbacks as the initial vote. Retries now
  correctly re-sink the card and trigger router.refresh on success
  (and roll back on error).

- governance-proposals-list + proposal-list-item: drop the parent
  divide-y in favor of a per-item border-b. divide-y put the missing
  divider on the first DOM item, but with CSS order reordering for
  optimistic vote sink that 'first' item could end up anywhere
  visually, producing missing/spurious dividers. Per-item borders
  follow the visual position.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// first DOM item would stay borderless even after being sunk to the visual
// bottom, producing missing/spurious dividers when an item is voted on.
return (
<div style={{ order }} className="border-b border-grey-01">

Copilot AI Apr 25, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProposalListItem always renders a bottom border, which will also add a divider after the final list item (unlike the previous divide-y behavior that only draws between items). If the extra trailing divider isn’t intended, consider using a last:border-b-0 (or equivalent) so the last visual item doesn’t show an extra border after reordering.

Suggested change
<div style={{ order }} className="border-b border-grey-01">
<div style={{ order }} className="border-b border-grey-01 last:border-b-0">

Copilot uses AI. Check for mistakes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

apps/web/app/home/home-proposals-infinite-scroll.tsx:43

  • HomeProposalsInfiniteScroll never resets loadMoreNodes, currentPageRef, or hasMore when governanceFilters / proposalType / connectedSpaceId / connectedAddress change. With the Suspense key removed, this component is likely to be preserved across navigations, so stale pages can be shown for a new filter set. Add an effect to clear state+refs when these inputs change, or key this component by the filter tuple.
export function HomeProposalsInfiniteScroll({
  connectedSpaceId,
  connectedAddress,
  proposalType,
  page = 0,
  initialHasMore = true,
  governanceFilters,
}: Props) {
  const sentinelRef = React.useRef<HTMLDivElement>(null);
  const [loadMoreNodes, setLoadMoreNodes] = React.useState<React.ReactNode[]>([]);
  const [isLoading, setIsLoading] = React.useState(false);
  const [hasMore, setHasMore] = React.useState(initialHasMore);

  const currentPageRef = React.useRef(page);
  const isLoadingRef = React.useRef(false);
  const hasMoreRef = React.useRef(initialHasMore);

apps/web/app/home/home-proposals-infinite-scroll.tsx:81

  • Because this effect aborts an in-flight loadMore call on cleanup, the current loadMore implementation can get stuck: when abortController.signal.aborted is true it skips resetting isLoadingRef.current / isLoading in finally. If props change while a page load is pending, pagination may be permanently blocked. Consider ensuring the ref is always reset even on abort, while still avoiding setState after unmount/abort.
  React.useEffect(() => {
    const signal = new AbortController();
    const element = sentinelRef.current;

    const observer = new IntersectionObserver(([entry]) => {
      if (entry.isIntersecting && !isLoadingRef.current && hasMoreRef.current) {
        loadMore(signal);
      }

apps/web/app/home/component.tsx:95

  • Removing the Suspense key means client component state below this boundary (notably HomeProposalsInfiniteScroll's loadMoreNodes / current page refs) can persist across filter changes. That can cause pages fetched for the previous filter to remain appended under the newly-rendered first page. Consider either reintroducing a key that changes with governanceFilters/proposalType/viewer, or adding explicit reset logic in the infinite-scroll component when those inputs change.
              <React.Suspense
                fallback={
                  <div className="space-y-2">
                    <LoadingSkeleton />
                    <LoadingSkeleton />
                  </div>
                }
              >
                <MyGovernanceProposalsList
                  memberSpaceId={connectedSpaceId}
                  viewerWalletAddress={connectedAddress}
                  spaceIds={myProposalSpaceIds}
                  spaceFilter={governanceFilters.spaceId}
                  category={governanceFilters.category}
                  status={governanceFilters.status}
                  governanceTab={governanceTab}
                  proposalType={proposalType}
                />
              </React.Suspense>
            ) : (
              <React.Suspense
                fallback={
                  <div className="space-y-2">
                    <LoadingSkeleton />
                    <LoadingSkeleton />
                    <LoadingSkeleton />
                  </div>
                }
              >
                <PendingProposals
                  connectedAddress={connectedAddress}
                  connectedSpaceId={connectedSpaceId}
                  proposalType={proposalType}
                  governanceFilters={governanceFilters}
                />
              </React.Suspense>
            )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 248 to 253
function apiProposalToGovernanceDto(
proposal: ApiProposalListItem,
bucket: ProposalBucket,
maybeProfile?: Profile,
maybeTargetProfile?: Profile
): GovernanceProposal {

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While touching apiProposalToGovernanceDto, note that proposal type is still inferred elsewhere in this function from only proposal.actions[0]. The REST schema warns action order isn't guaranteed; multi-action proposals can be misclassified (e.g. any PUBLISH should force ADD_EDIT). Consider switching to mapApiActionsToProposalType(proposal.actions) to keep filtering/display correct.

Copilot uses AI. Check for mistakes.
Comment on lines +281 to +285
function getProposalBucket(apiStatus: string): ProposalBucket {
if (apiStatus === 'EXECUTABLE') return 'executable';
if (apiStatus === 'PROPOSED') return 'active';
return 'completed';
}

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getProposalBucket takes apiStatus: string, which loses the REST schema’s union type and makes it easy to pass an invalid status (silently mapping to completed). Consider typing this as ApiProposalListItem['status'] (or ApiProposalStatusResponse['status']) and handling the union exhaustively so new API statuses don’t get mis-bucketed by default.

Copilot uses AI. Check for mistakes.
Address the two new Copilot review comments on the apiProposalToGovernanceDto path:

- Switch from mapActionTypeToProposalType(actions[0]) to
  mapApiActionsToProposalType(actions). The REST schema warns that
  action order isn't guaranteed, so a multi-action proposal whose
  PUBLISH lives at index > 0 was being misclassified — affecting
  filter/category routing and display naming.

- Narrow getProposalBucket's parameter from string to
  ApiProposalListItem['status'] and exhaustive-switch the four cases.
  Previously an arbitrary or new status silently fell through to
  'completed'; now any future status added to the schema is a type
  error here. The runtime behavior is unchanged for the existing
  PROPOSED/EXECUTABLE/ACCEPTED/REJECTED values.

Skipped Copilot's last:border-b-0 suggestion on ProposalListItem:
last-child targets DOM-last, not visually-last. With CSS order
sinking voted cards, applying border-b-0 to DOM-last would drop the
divider in the middle of the visual list. The trailing border on
the actual visual bottom is the lesser evil.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

apps/web/app/home/home-proposals-infinite-scroll.tsx:43

  • HomeProposalsInfiniteScroll keeps loadMoreNodes, currentPageRef, and hasMoreRef across prop changes. Now that the Suspense key was removed upstream, changing filters / proposalType can reuse the same component instance and cause pages from the previous query to be appended to the new list (and pagination continuing from the old currentPageRef.current). Add an effect to reset state/refs when connectedSpaceId, connectedAddress, proposalType, governanceFilters, or page/initialHasMore change.
  const sentinelRef = React.useRef<HTMLDivElement>(null);
  const [loadMoreNodes, setLoadMoreNodes] = React.useState<React.ReactNode[]>([]);
  const [isLoading, setIsLoading] = React.useState(false);
  const [hasMore, setHasMore] = React.useState(initialHasMore);

  const currentPageRef = React.useRef(page);
  const isLoadingRef = React.useRef(false);
  const hasMoreRef = React.useRef(initialHasMore);

apps/web/partials/governance/governance-proposals-list.tsx:412

  • Proposal-type filtering still checks only p.actions[0]?.actionType, but earlier in this file we now treat REST action order as non-deterministic (see mapApiActionsToProposalType). If the membership action isn't at index 0, content vs request filtering will be wrong. Use p.actions.some(a => MEMBERSHIP_ACTION_TYPES.has(a.actionType)) (and its inverse) instead of relying on actions[0].
  // Filter by proposal type
  if (effectiveType === 'proposals') {
    combinedProposals = combinedProposals.filter(p => !MEMBERSHIP_ACTION_TYPES.has(p.actions[0]?.actionType ?? ''));
  } else if (effectiveType === 'requests') {
    combinedProposals = combinedProposals.filter(p => MEMBERSHIP_ACTION_TYPES.has(p.actions[0]?.actionType ?? ''));
  }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 426 to +446
// Fetch target profiles for membership proposals (extract targetId from actions)
const targetIds = paginatedProposals
.filter(p => MEMBERSHIP_ACTION_TYPES.has(p.actions[0]?.actionType ?? ''))
.map(p => p.actions[0]?.targetId)
.filter((id): id is string => !!id);
const uniqueTargetIds = [...new Set(targetIds)];

const [profilesForProposals, profilesForTargets] = await Promise.all([
Effect.runPromise(fetchProfilesBySpaceIds(uniqueProposedByIds)),
uniqueTargetIds.length > 0 ? Effect.runPromise(fetchProfilesBySpaceIds(uniqueTargetIds)) : [],
]);

// Create maps for efficient lookup
const profilesBySpaceId = new Map(uniqueProposedByIds.map((id, i) => [id, profilesForProposals[i]]));
const targetProfilesBySpaceId = new Map(uniqueTargetIds.map((id, i) => [id, profilesForTargets[i]]));

const proposals = paginatedProposals.map(p => {
const maybeProfile = profilesBySpaceId.get(p.proposedBy);
const targetId = p.actions[0]?.targetId;
const maybeTargetProfile = targetId ? targetProfilesBySpaceId.get(targetId) : undefined;
return apiProposalToGovernanceDto(p, maybeProfile, maybeTargetProfile);
return apiProposalToGovernanceDto(p, getProposalBucket(p.status), maybeProfile, maybeTargetProfile);

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Target-profile resolution for membership proposals assumes the membership action is actions[0] (both when collecting targetIds and when reading targetId for a specific proposal). Since REST action order isn't guaranteed, this can fail to resolve the target profile or resolve the wrong one. Find the membership action in actions and read its targetId instead of hard-coding index 0.

Copilot uses AI. Check for mistakes.
const addOptimisticVote = useAddOptimisticVote();
const removeOptimisticVote = useRemoveOptimisticVote();

const onVoteSuccess = () => {

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useAddOptimisticVote entries are only removed on error; on success they remain in the atom indefinitely. That means the item can keep getting the artificial CSS order bump even after the server data reflects the vote, and the set can grow without bound in a long session. Consider clearing the optimistic flag in onVoteSuccess (or once refreshed data shows userVote) before/after router.refresh().

Suggested change
const onVoteSuccess = () => {
const onVoteSuccess = () => {
removeOptimisticVote(proposalId);

Copilot uses AI. Check for mistakes.
Comment on lines 49 to +59
const { smartAccount } = useSmartAccount();
const addOptimisticVote = useAddOptimisticVote();
const removeOptimisticVote = useRemoveOptimisticVote();

const onVoteSuccess = () => {
router.refresh();
};

const onVoteError = () => {
removeOptimisticVote(proposalId);
};

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useAddOptimisticVote entries are only removed on error; on success they remain in the atom indefinitely. That can keep applying the CSS order bump even after the vote is reflected by refreshed server data, and it grows the set over time. Consider removing the optimistic flag in onVoteSuccess (or when the component observes userVote) in addition to calling router.refresh().

Copilot uses AI. Check for mistakes.
…on userVote

Two more Copilot review items addressed:

- governance-proposals-list: replace four index-0 reads on
  proposal.actions with a findMembershipAction(actions) helper.
  REST action order isn't guaranteed, so multi-action proposals
  with a membership action at index > 0 were being missed by both
  the type filter (proposals vs requests) and the target-profile
  resolution path. The latter was leaving the proposer's name
  showing as the target on those cards.

- accept-or-reject + accept-or-reject-editor + accept-or-reject-member:
  add a useEffect that removes the optimistic entry from the atom
  once the userVote prop arrives from the server. Previously the set
  grew across a session and the +5000 CSS order bump kept applying
  even after server data already placed the card at the end of its
  bucket. The cleanup is safe from a flash perspective because at
  the moment the bump is removed, the server's render has already
  put the voted card at the bucket-end position.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jwalkingjew
jwalkingjew merged commit 8a76779 into master Apr 28, 2026
4 checks passed
@jwalkingjew
jwalkingjew deleted the governance-ux-polish branch April 28, 2026 18:28

This branch was successfully deployed

1 active deployment
Preview — 3482eb32 Deployed Apr 28, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants