Fix(ui): Keep My Trades input visible while typing - #112
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe message input now shows the latest grapheme-aligned text that fits its visible width. The truncation logic handles Unicode display widths, combining characters, and ZWJ emoji sequences. Tests cover calculation and rendered output. ChangesChat input rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: 🔵 Low · up to The input now keeps trailing text visible while typing, but some decomposed characters or joined emoji sequences may render partially or incorrectly at the left edge when text overflows. The PR is mergeable with owner awareness and follow-up for Unicode grapheme-safe truncation. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/ui/tabs/order_in_progress_tab.rs`:
- Around line 160-169: Update the truncation logic around order_chat_input to
iterate with UnicodeSegmentation::grapheme_indices(true) instead of
char_indices(), ensuring the selected suffix always begins on a grapheme
boundary, including decomposed characters and ZWJ emoji sequences. Add
unicode-segmentation as a direct dependency and add regression tests covering
both cases.
- Around line 703-732: Add a deterministic TestBackend integration test
alongside the existing trailing input tests, initializing my_trades_maker_book
with one row and setting app.order_chat_input to text exceeding main_chunks[2]’s
inner width. Render render_order_in_progress to a fixed-size terminal and assert
the terminal buffer contains the expected trailing suffix, covering the
Block::inner and Paragraph rendering path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d9ec61e1-c53e-462d-a9d7-2b161ef9972a
📒 Files selected for processing (1)
src/ui/tabs/order_in_progress_tab.rs
|
Addressed review feedback in the latest commit.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Review: approved
The current head addresses the previous feedback: input truncation now walks grapheme boundaries, adds unicode-segmentation as a direct dependency, and includes focused helper tests plus a TestBackend render regression for the overflowing My Trades input.
I did not find blocking issues in the diff.
Verified locally on b5fd811e3243c3cd74adb7ecf37c40b1b1f3bc65:
cargo fmt --all -- --check✅cargo test order_in_progress_tab::tests --all-features✅cargo check --all-targets --all-features✅cargo clippy --all-targets --all-features -- -D warnings✅cargo test --all-features✅
Review findings on MostroP2P#114, all three verified against the code before fixing: 1. Security (CodeRabbit): the restore response sender was discarded. Unlike the order flows there is no random request_id an attacker cannot know — new_restore sends none — so on GiftWrap, where the subscription cannot filter by author (ephemeral wrap signers), a forged identity-encrypted RestoreData could seed attacker-controlled orders and advance last_trade_index. The response is now rejected unless the unwrapped sender is the configured Mostro pubkey. 2. Maker/taker (ermeme): restored rows all claimed taker. The exact inference suggested (trade pubkey vs buyer/seller pubkey) is not possible from public data — kind-38383 tags stop at the order terms — but the protocol allows a partial one: Pending / WaitingMakerBond orders exist only for their maker, so those now restore as maker. Genuinely ambiguous rows still fall back to taker, but are counted in the summary ("N order(s) restored with unknown maker/taker role") so the fallback is never silent. 3. UI projections (ermeme): the restore task rewrote SQLite but sent a plain Info result, which does not re-run the DB-to-UI sync — recovered orders stayed invisible until restart. The task now sends a dedicated OperationResult::SessionRestored, and apply_order_result runs the same startup sync pair (refresh_my_trades_maker_book_cache + sync_user_order_history_messages_from_db) before showing the popup. handle_operation_result normalizes the variant to Info for display. Also rebased onto main (mergeable_state was dirty after MostroP2P#106/MostroP2P#112 landed). Tests: role inference (maker-exclusive statuses vs ambiguous ones), the resync trigger matrix, and summary wording for unknown roles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
This PR fixes the My Trades order chat message input when typed text exceeds the visible field width.
Testing
cargo fmt --all -- --checkcargo check --all-targets --all-featurescargo test --all-featurescargo clippy --all-targets --all-features -- -D warningsAll checks passed successfully.
Summary by CodeRabbit
Bug Fixes
Tests