Skip to content

feature: hide failed tx from transaction history#810

Open
ax-x2 wants to merge 3 commits intosolana-foundation:masterfrom
ax-x2:feature/hide-failed-tx
Open

feature: hide failed tx from transaction history#810
ax-x2 wants to merge 3 commits intosolana-foundation:masterfrom
ax-x2:feature/hide-failed-tx

Conversation

@ax-x2
Copy link

@ax-x2 ax-x2 commented Jan 15, 2026

Description

adds a toggle button next to the refresh button that allows users to hide failed transactions from the transaction history view. filter state persists across sessions using localstorage.

Type of change

  • Bug fix
  • New feature
  • Protocol integration
  • Documentation update
  • Other (please describe):

Screenshots

Screenshot_20260115_161435

Checklist

  • My code follows the project's style guidelines
  • I have added tests that prove my fix/feature works
  • All tests pass locally and in CI
  • I have updated documentation as needed
  • I have run build:info script to update build information
  • CI/CD checks pass
  • I have included screenshots for protocol screens (if applicable)
  • For security-related features, I have included links to related information

Important

Add feature to hide failed transactions in history with a toggle button, persisting state using localStorage.

  • UI Changes:
    • Add toggle button in HistoryCardHeader to hide/show failed transactions.
    • Button state persists using localStorage via useHideFailedTransactions hook.
  • Functionality:
    • TransactionHistoryCard filters transactions based on toggle state.
    • getTransactionRows processes transactions, marking them as failed or successful.
  • Testing:
    • Add tests in HistoryCardComponents.spec.tsx for toggle button behavior.
    • Add tests in useHideFailedTransactions.spec.tsx for hook functionality and persistence.

This description was created by Ellipsis for 3ecb10d. You can customize this summary. It will automatically update as commits are pushed.

@vercel
Copy link

vercel bot commented Jan 15, 2026

@ax-x2 is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Important

Looks good to me! 👍

Reviewed everything up to 3ecb10d in 2 minutes and 9 seconds. Click for details.
  • Reviewed 593 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 6 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. app/components/account/HistoryCardComponents.tsx:31
  • Draft comment:
    Consider moving inline flex styling (display: 'flex', gap: '8px') to a CSS class for consistency and maintainability.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
2. app/components/account/HistoryCardComponents.tsx:36
  • Draft comment:
    The onClick uses '!hideFailedTxs' to toggle the state. This is acceptable but if asynchronous state updates become a concern, consider using a functional updater.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
3. app/components/account/history/TransactionHistoryCard.tsx:34
  • Draft comment:
    The useEffect hook depends only on 'address'. Excluding refresh (or history) may lead to stale closures. Confirm if this dependency omission is intentional.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
4. app/components/account/history/TransactionHistoryCard.tsx:87
  • Draft comment:
    Instead of wrapping 'refresh' in an arrow function (e.g., refresh={() => refresh()}), consider passing 'refresh' directly to simplify the code.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
5. app/hooks/useHideFailedTransactions.ts:29
  • Draft comment:
    Good use of lazy initialization and SSR-safe check. Optionally, consider logging error details in the catch blocks for better debugging.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50% None
6. app/hooks/useHideFailedTransactions.ts:11
  • Draft comment:
    The function JSDoc specifies the return as [hideFailedTxs, setHideFailedTxs], but the actual returned setter is named setHideFailedTxsState. Consider updating the documentation for consistency.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment is technically correct - there is an inconsistency between the JSDoc and the implementation. However, I need to consider: (1) Is this an important issue? The JSDoc is describing what the return value represents conceptually, not necessarily the exact variable names used internally. (2) The actual return type is correctly specified in TypeScript as [boolean, (value: boolean) => void], so there's no functional issue. (3) This is a very minor documentation inconsistency that doesn't affect functionality. (4) The JSDoc is arguably correct as-is since it's describing the semantic meaning of the return values, not the internal variable names. (5) This feels like a nitpick that may not be worth addressing. The comment is factually accurate and documentation consistency is generally good practice. JSDoc comments often do reference actual variable names for clarity, so this could be a legitimate improvement. The comment provides a clear, actionable suggestion. While the comment is factually correct, the JSDoc is describing the conceptual return values for users of the hook, not documenting internal implementation details. The name setHideFailedTxs in the JSDoc is actually more user-friendly and clearer than setHideFailedTxsState. This is a very minor issue that doesn't affect functionality or understanding. This comment is too minor and nitpicky. The JSDoc is describing the semantic meaning of the return values for users of the hook, and using setHideFailedTxs is actually clearer than the internal variable name. The inconsistency doesn't cause confusion or functional issues. This falls under "obvious or unimportant" comments that should be removed.

Workflow ID: wflow_xcnLRI9EwWYtILHo

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

@rogaldh
Copy link
Contributor

rogaldh commented Jan 15, 2026

@ax-x2 Hi, could you please collect the build:info for this feature? Check the script inside package.json

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.

3 participants