Skip to content

UX improvement - #57

Closed
dewabisma wants to merge 3 commits into
mainfrom
beast/ux-improvement
Closed

dewabisma wants to merge 3 commits into
mainfrom
beast/ux-improvement

Conversation

@dewabisma

Copy link
Copy Markdown
Collaborator

No description provided.

@dewabisma
dewabisma requested a review from n13 October 5, 2026 06:44

@n13 n13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewer model: GPT-6.1 Sol

REQUEST_CHANGES — keep transfer outcomes visible when simplifying the transaction tables.

[P2] Preserve the status indicator for unified transaction rows — UNIFIED_LIST_TRANSACTION_COLUMNS.tsx:97–102

The list queries still include scheduled, executed, and cancelled reversible transfers, but deleting the Status cell leaves no visible outcome or transaction-type indicator in the global, account, and multisig transaction tables. In account activity, createAccountUnifiedTransactionColumns also continues to render a green IN badge and the amount for a cancelled or merely scheduled incoming transfer. A base/head rendering comparison with otherwise identical fixtures confirms that all three outcomes now have identical visible cells (IN, 1 QTC, etc.); the base shows Scheduled, Executed, or Cancelled. This makes a transfer that never completed look like a completed credit. Retain an outcome badge, either as a column or within an existing cell, so users can distinguish completed transfers from pending/cancelled ones without opening each detail page.

Reviewed base ec933ec → head 63b37c5, including all 19 changed files and affected callers.

Validation:

  • Frozen-lockfile dependency installation, production build/TypeScript check, git diff --check, and Prettier checks for all changed files passed.
  • Actual base/head column-rendering reproduction confirmed the outcome loss described above.
  • Raw bun test: 173 passed, 3 failed from DOM cleanup in unchanged component suites. With a temporary afterEach(cleanup) preload, all 176 tests passed; no tracked source files were changed.
  • Both GitHub checks passed. The workflow named “Build and Test” runs the build; its test step is disabled.

@dewabisma
dewabisma requested a review from n13 October 5, 2026 08:11

@n13 n13 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewer model: Opus 5.5

Nothing here blocks the merge. tsc --noEmit passes on the PR head. The lint errors in the touched files are all on lines this PR did not change. Jest cannot run in this repo right now. jest.setup.ts fails to parse in every suite, and that problem is older than this PR.

Findings, most severe first:

  1. src/components/common/table-columns/EXTRINSIC_COLUMNS.tsx:93. The block's extrinsic list no longer shows which extrinsics failed, because the Result column is gone. createExtrinsicColumns only feeds the block detail table, and nothing else in that table marks a failure. Users now have to open each extrinsic to find a failed one. If this is intended, also drop success from GetBlockById (src/api/blocks.tsx:81) and BlockExtrinsic (src/schemas/blocks.ts:20). Nothing reads it now.

  2. src/components/features/multisig-details/MultisigDetailStatusFilter.tsx:21. Clicking the status button that is already active now breaks scroll restore on Back. The URL does not change, so router-core takes its same-URL branch. That branch still sets resetNextScroll = false. onRendered does not fire, so the flag is never reset. On the next browser Back, the scroll handler sees the stale flag and skips restoreScroll. The previous page then keeps the wrong scroll position. Fix: return early when nextStatus === status. The tabs are not affected, because Radix does not fire onValueChange for the active tab.

  3. src/components/features/multisig-listing/MultisigDataTabs.tsx:28 and MultisigStatusFilter.tsx:26. The /multisig listing page has the same tab and status navigation, with MultisigStats above it. It did not get resetScroll: false, so switching tab or status there still jumps to the top. These two components are near copies of the detail ones. One shared implementation would stop the fixes from drifting apart.

  4. src/components/common/table-columns/UNIFIED_LIST_TRANSACTION_COLUMNS.tsx:122. Without the fallback, an unknown status throws Cannot read properties of undefined (reading 'variant'). The router error page then replaces the whole transactions page. Hasura types status as String, so the TS union is only a cast. The indexer enum has exactly these five values today, so nothing triggers it yet. Failing loudly is fine, but throw an error that names the status so the cause is clear. Note that MULTISIG_PROPOSAL_COLUMNS.tsx:29 still falls back to 'miner', so the two tables now behave differently.

  5. Removed sortable columns still sort. useOrderBy passes any order_by from the URL straight to GraphQL. An old link with ?order_by=fee:desc, deposit:asc or expiry_block:desc still sorts by a column the user cannot see, and no header shows an arrow. This mechanism existed before, but this PR turns valid links into hidden sorts. Remove fee from src/constants/query-sorts/unified-list-transactions.ts:6,19,29 and deposit/expiry_block from src/constants/query-sorts/multisig-proposal.ts:6,8,15,19. The *_SORTS_LITERALS arrays are not imported anywhere, so they validate nothing.

  6. Unused fields are still fetched. fee in UNIFIED_TX_FIELDS (src/api/unified-transactions.tsx:40) goes into every unified list query and the landing page, and nothing reads it. deposit and expiry_block in MultisigProposalListFields (src/api/multisig-proposals.tsx:19-20) are now only needed by the detail fragment.

  7. src/components/ui/composites/account-address-cell/AccountAddressCell.tsx:42. truncate = true is now the minority case. 46 call sites pass truncate={false}, and the 19 that use the default are all in table columns. Flip the default to false to match LinkWithCopy, and pass truncate in the table columns. Otherwise every new detail page has to remember the flag. The same "cell or EmptyValue" JSX is also repeated about 22 times across the detail pages. accountLink and walletLink in MultisigProposalInformation.tsx:80-98 already wrap it. Moving them to a shared module would have made this a two-line change.

  8. Some classes now do nothing. className="font-mono text-flare" at UNIFIED_LIST_TRANSACTION_COLUMNS.tsx:83,100 repeats LinkWithCopy's defaults, so the prop can go. className="break-all" next to the new truncate={false} is also redundant, because LinkWithCopy already adds break-all when truncation is off. This affects the reversible-transaction, miner-reward and deposits-claimed pages.

Verdict: comment. There are no blocking bugs. Please confirm that hiding extrinsic failures on the block page is intended, and consider the one-line fix for #2.

@dewabisma

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #58

@dewabisma dewabisma closed this Oct 5, 2026
@dewabisma
dewabisma deleted the beast/ux-improvement branch October 5, 2026 10:18
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