Skip to content

Revert summary separator rendering changes#1385

Merged
jjramirezn merged 1 commit intopeanut-wallet-devfrom
revert-summary-separator-rendering
Nov 3, 2025
Merged

Revert summary separator rendering changes#1385
jjramirezn merged 1 commit intopeanut-wallet-devfrom
revert-summary-separator-rendering

Conversation

@Zishan-7
Copy link
Contributor

No description provided.

@vercel
Copy link

vercel bot commented Oct 31, 2025

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

Project Deployment Preview Comments Updated (UTC)
peanut-wallet Ready Ready Preview Comment Oct 31, 2025 2:47pm

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Walkthrough

The TransactionDetailsReceipt component was refactored to replace grouped-border visibility logic with simpler single-criteria border logic. Multiple PaymentInfoRow instances now use shouldHideBorder to determine border visibility based on whether a row is the global last visible row, instead of group-level last-in-group handling. A TODO comment indicates future work to re-enable grouped borders.

Changes

Cohort / File(s) Summary
Border Logic Refactoring
src/components/TransactionDetails/TransactionDetailsReceipt.tsx
Replaced shouldHideGroupBorder with shouldHideBorder across multiple PaymentInfoRow instances; removed group-level last-in-group border handling in favor of global last-visible-row logic; added TODO comment for future grouped border re-enablement

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Single file affected with repetitive pattern changes (function call replacement)
  • Logic is straightforward but requires understanding the border visibility implications
  • TODO comment suggests incomplete work that may need clarification

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • Hugo0
  • jjramirezn

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description Check ❓ Inconclusive No pull request description was provided by the author. While the check is intentionally lenient and passes as long as the description is not completely off-topic, the complete absence of a description makes it impossible to evaluate whether it relates to the changeset. This creates ambiguity about whether the lack of content should be considered a pass or a failure under the lenient criteria. Consider adding a brief description explaining the purpose of reverting the summary separator rendering changes, such as what issue or regression motivated this revert or any relevant context for reviewers.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title "Revert summary separator rendering changes" is directly related to the changeset, which modifies border rendering logic in the TransactionDetailsReceipt component. The changes replace grouped-border logic with single-criteria border logic, affecting how separators (borders) are rendered in PaymentInfoRow instances. The title accurately conveys the main change and is specific enough that a reviewer scanning history would understand the nature of the modification.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch revert-summary-separator-rendering

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added the enhancement New feature or request label Oct 31, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (1)

246-259: Consider removing unused grouped-border infrastructure or documenting the timeline for re-enablement.

The shouldHideGroupBorder function and its supporting infrastructure (rowGroups, lastVisibleInGroups, getLastVisibleInGroup) are defined but unused after this refactor. While the TODO comment indicates these will be re-enabled, keeping unused code can:

  • Increase cognitive load for maintainers
  • Risk divergence if the main logic evolves before re-enablement
  • Cause confusion about which helpers are actually in use

Consider either:

  1. Removing this infrastructure now and restoring it when tackling grouped borders
  2. Adding a more specific TODO with a timeline or issue reference
  3. Adding JSDoc comments to clarify these are intentionally unused pending future work
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2655492 and 817bcb4.

📒 Files selected for processing (1)
  • src/components/TransactionDetails/TransactionDetailsReceipt.tsx (5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-24T12:38:32.793Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 478
File: src/components/Request/Create/Views/Initial.view.tsx:81-89
Timestamp: 2024-10-24T12:38:32.793Z
Learning: In `src/components/Request/Create/Views/Initial.view.tsx`, the function `getTokenDetails` is a simple function that does not fetch from the network or perform asynchronous operations.

Applied to files:

  • src/components/TransactionDetails/TransactionDetailsReceipt.tsx
🧬 Code graph analysis (1)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (2)
src/components/Payment/PaymentInfoRow.tsx (1)
  • PaymentInfoRow (18-87)
src/utils/general.utils.ts (1)
  • formatDate (522-534)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Deploy-Preview
🔇 Additional comments (1)
src/components/TransactionDetails/TransactionDetailsReceipt.tsx (1)

601-601: Consistent simplification of border logic across all affected rows.

The replacement of shouldHideGroupBorder with shouldHideBorder is applied consistently across all PaymentInfoRow instances. This simplifies the separator rendering from group-aware logic to a single criterion (global last visible row), which aligns with the PR objective.

Note: This changes the visual appearance by removing group separators between logical sections (dateRows, txnDetails, fees). All rows except the final visible row will now show borders.

Also applies to: 609-609, 617-617, 625-625, 709-709, 736-736, 1091-1091, 1099-1099

@jjramirezn jjramirezn merged commit 4aa61b5 into peanut-wallet-dev Nov 3, 2025
5 checks passed
@jjramirezn jjramirezn deleted the revert-summary-separator-rendering branch November 3, 2025 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants