-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: add memo to tx info modal #147
Conversation
WalkthroughThe pull request introduces optional Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #147 +/- ##
==========================================
- Coverage 57.05% 57.02% -0.03%
==========================================
Files 145 145
Lines 14419 14433 +14
==========================================
+ Hits 8227 8231 +4
- Misses 6192 6202 +10 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (1)
hooks/useQueries.ts (1)
808-811
: Add test coverage for memo handlingThe new memo handling code lacks test coverage. Consider adding tests for:
- Memo extraction from transaction body
- Memo propagation through message transformation
- Different transaction types with memos
Would you like me to help create test cases for the memo functionality?
Also applies to: 823-823
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 808-811: hooks/useQueries.ts#L808-L811
Added lines #L808 - L811 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
components/bank/components/historyBox.tsx
(1 hunks)components/bank/modals/txInfo.tsx
(1 hunks)hooks/useQueries.ts
(8 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
hooks/useQueries.ts
[warning] 717-717: hooks/useQueries.ts#L717
Added line #L717 was not covered by tests
[warning] 732-732: hooks/useQueries.ts#L732
Added line #L732 was not covered by tests
[warning] 742-742: hooks/useQueries.ts#L742
Added line #L742 was not covered by tests
[warning] 752-752: hooks/useQueries.ts#L752
Added line #L752 was not covered by tests
[warning] 773-773: hooks/useQueries.ts#L773
Added line #L773 was not covered by tests
[warning] 783-783: hooks/useQueries.ts#L783
Added line #L783 was not covered by tests
[warning] 808-811: hooks/useQueries.ts#L808-L811
Added lines #L808 - L811 were not covered by tests
[warning] 823-823: hooks/useQueries.ts#L823
Added line #L823 was not covered by tests
🔇 Additional comments (3)
components/bank/components/historyBox.tsx (1)
16-16
: LGTM: Interface changes for memo support
The optional memo fields are correctly added to both interfaces, maintaining backward compatibility while enabling memo support.
Also applies to: 24-24
hooks/useQueries.ts (1)
717-717
: LGTM: Consistent memo handling across message types
The memo field is consistently added to all transaction types in the _formatMessage function.
Also applies to: 732-732, 742-742, 752-752, 773-773, 783-783
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 717-717: hooks/useQueries.ts#L717
Added line #L717 was not covered by tests
components/bank/modals/txInfo.tsx (1)
93-97
: LGTM: Clean implementation of memo display
The memo display is well-implemented:
- Uses existing InfoItem component for consistent UI
- Properly handles optional memo with conditional rendering
- Logical placement at the bottom of the modal
Superseded by #140 |
fixes #140
Summary by CodeRabbit
New Features
Bug Fixes