feat(ui): redesign Create New Order tab with picker and leave guard#93
Open
arkanoider wants to merge 2 commits into
Open
feat(ui): redesign Create New Order tab with picker and leave guard#93arkanoider wants to merge 2 commits into
arkanoider wants to merge 2 commits into
Conversation
Replace the sparse stacked-field form with a sectioned layout, live preview receipt, inline validation, and a searchable currency dropdown backed by instance fiat_currencies_accepted or a bundled ISO list. Add draft persistence and a confirm-on-leave popup when switching tabs with unsaved input; allow spaces in payment method descriptions. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
👮 Files not reviewed due to content moderation or server errors (3)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/ui/currencies.rs (1)
251-272: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider deduplicating/filtering blank codes in
resolve_options.If
acceptedcontains duplicate or empty codes (e.g. malformed relay data), the resulting option list will contain duplicate or blank entries in the picker dropdown.♻️ Suggested defensive fix
pub fn resolve_options(accepted: &[String]) -> Vec<CurrencyOption> { if accepted.is_empty() { return CURRENCIES .iter() .map(|c| CurrencyOption { code: c.code.to_string(), name: c.name.to_string(), }) .collect(); } - accepted + let mut seen = std::collections::HashSet::new(); + accepted .iter() - .map(|code| { - let upper = code.trim().to_ascii_uppercase(); - CurrencyOption { - name: name_for(&upper).to_string(), - code: upper, - } - }) + .filter_map(|code| { + let upper = code.trim().to_ascii_uppercase(); + if upper.is_empty() || !seen.insert(upper.clone()) { + return None; + } + Some(CurrencyOption { + name: name_for(&upper).to_string(), + code: upper, + }) + }) .collect() }🤖 Prompt for 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. In `@src/ui/currencies.rs` around lines 251 - 272, resolve_options currently maps every accepted entry directly into a CurrencyOption, so duplicate or blank inputs can produce duplicate or empty picker items. Update resolve_options to defensively normalize each code, skip entries that trim to empty, and deduplicate by the uppercased currency code before collecting. Keep the behavior for the empty-accepted case unchanged, and make sure the filtering happens in the resolve_options path using name_for and CurrencyOption.src/ui/key_handler/navigation.rs (1)
505-539: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding unit tests for the new draft/leave-guard state machine.
restore_or_new_form,handle_form_leave_attempt, andleave_creating_order_to_adjacent_tabintroduce non-trivial branching (dirty vs. clean form, direction, draft take/restore) split across this file plusesc_handlers.rs/confirmation.rs/enter_handlers.rs. A few focused unit tests aroundhandle_form_leave_attempt(dirty →ConfirmLeaveOrder, clean → immediate leave with draft cleared) would guard against regressions as this flow evolves.🤖 Prompt for 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. In `@src/ui/key_handler/navigation.rs` around lines 505 - 539, Add focused unit tests for the new draft/leave-guard flow around restore_or_new_form, handle_form_leave_attempt, and leave_creating_order_to_adjacent_tab. Cover the key branches: dirty form should switch AppState.mode to UserMode::ConfirmLeaveOrder with the form preserved, clean form should clear order_form_draft and immediately call leave_creating_order_to_adjacent_tab, and restore_or_new_form should consume a saved draft via take() or fall back to FormState::new_default_form. Use these symbols in the tests so the branching behavior stays covered even if surrounding handlers in esc_handlers.rs, confirmation.rs, or enter_handlers.rs change.src/ui/key_handler/form_input.rs (1)
15-121: 📐 Maintainability & Code Quality | 🔵 TrivialConsider adding unit tests for the new picker state machine.
handle_currency_picker_key,open_currency_picker, andclose_currency_pickerintroduce non-trivial branching (open/close, filter mutation, wraparound Up/Down index math, selection clamping on filter change) with no accompanying tests in this file, unlike the existingis_creating_order_text_inputtest below it.🤖 Prompt for 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. In `@src/ui/key_handler/form_input.rs` around lines 15 - 121, Add unit tests for the new picker state machine around handle_currency_picker_key, open_currency_picker, and close_currency_picker. Cover closed-vs-open behavior, opening via Enter/Space/typing, filter mutation and reset, Up/Down wraparound, Enter selection clamping after filtering, and Esc closing/resetting state. Use the existing CreatingOrder/FormState/AppState paths and mirror the style of the is_creating_order_text_input test below it.
🤖 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 `@docs/TUI_INTERFACE.md`:
- Around line 345-357: Add a language specifier to the fenced code block in the
“Key files for codegen” section to satisfy MD040; update the markdown fence
around the file list in docs/TUI_INTERFACE.md to include a language tag such as
text, keeping the contents and surrounding documentation unchanged.
In `@src/ui/order_form.rs`:
- Around line 352-452: The focused cursor position is derived from Row.text_len
in render_details, but Amount, Premium, and Expiry are rendered with extra
decoration from amount_val, premium_line, and expiry_line so the caret no longer
matches the visible text. Update the Row construction in order_form.rs so these
fields show the raw editable string whenever form.focused is
FormField::AmountSats, FormField::Premium, or FormField::ExpirationDays, and
keep the decorated display only when unfocused. This will make text_len align
with what is actually rendered and fix the cursor placement.
- Around line 651-732: The popup sizing in the currency selection render path
can leave the empty-state message with no space when filtered is empty. Update
the sizing logic in the render block around the popup Rect/Layout split so the
inner area always reserves enough rows for both the "no match" text and the
hint, and make the height calculation in the currency popup rendering robust
against the zero-item case. Use the existing symbols filtered, max_rows, popup,
and the [Constraint::Min(1), Constraint::Length(1)] split to locate the fix.
---
Nitpick comments:
In `@src/ui/currencies.rs`:
- Around line 251-272: resolve_options currently maps every accepted entry
directly into a CurrencyOption, so duplicate or blank inputs can produce
duplicate or empty picker items. Update resolve_options to defensively normalize
each code, skip entries that trim to empty, and deduplicate by the uppercased
currency code before collecting. Keep the behavior for the empty-accepted case
unchanged, and make sure the filtering happens in the resolve_options path using
name_for and CurrencyOption.
In `@src/ui/key_handler/form_input.rs`:
- Around line 15-121: Add unit tests for the new picker state machine around
handle_currency_picker_key, open_currency_picker, and close_currency_picker.
Cover closed-vs-open behavior, opening via Enter/Space/typing, filter mutation
and reset, Up/Down wraparound, Enter selection clamping after filtering, and Esc
closing/resetting state. Use the existing CreatingOrder/FormState/AppState paths
and mirror the style of the is_creating_order_text_input test below it.
In `@src/ui/key_handler/navigation.rs`:
- Around line 505-539: Add focused unit tests for the new draft/leave-guard flow
around restore_or_new_form, handle_form_leave_attempt, and
leave_creating_order_to_adjacent_tab. Cover the key branches: dirty form should
switch AppState.mode to UserMode::ConfirmLeaveOrder with the form preserved,
clean form should clear order_form_draft and immediately call
leave_creating_order_to_adjacent_tab, and restore_or_new_form should consume a
saved draft via take() or fall back to FormState::new_default_form. Use these
symbols in the tests so the branching behavior stays covered even if surrounding
handlers in esc_handlers.rs, confirmation.rs, or enter_handlers.rs change.
🪄 Autofix (Beta)
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
Run ID: 4af1b141-fa7d-4019-bfb9-4ef26022e8c9
📒 Files selected for processing (16)
docs/README.mddocs/TUI_INTERFACE.mdsrc/ui/app_state.rssrc/ui/currencies.rssrc/ui/draw.rssrc/ui/key_handler/confirmation.rssrc/ui/key_handler/enter_handlers.rssrc/ui/key_handler/esc_handlers.rssrc/ui/key_handler/form_input.rssrc/ui/key_handler/mod.rssrc/ui/key_handler/navigation.rssrc/ui/mod.rssrc/ui/order_confirm.rssrc/ui/order_form.rssrc/ui/orders.rssrc/ui/user_state.rs
Comment on lines
+345
to
+357
| #### Key files for codegen | ||
|
|
||
| ``` | ||
| src/ui/order_form.rs # render_order_form, validation, preview | ||
| src/ui/currencies.rs # ISO list + resolve/filter helpers | ||
| src/ui/orders.rs # FormState, FormField, CurrencyPicker, is_dirty | ||
| src/ui/order_confirm.rs # render_order_confirm, render_leave_confirm | ||
| src/ui/draw.rs # tab render + ConfirmLeaveOrder overlay | ||
| src/ui/key_handler/form_input.rs # char/backspace + handle_currency_picker_key | ||
| src/ui/key_handler/navigation.rs # tab leave guard, restore_or_new_form | ||
| src/ui/key_handler/enter_handlers.rs # ConfirmingOrder / ConfirmLeaveOrder Enter | ||
| src/ui/key_handler/esc_handlers.rs # Esc on form / leave guard | ||
| ``` |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language to the fenced code block (MD040).
Static analysis flags this fence for missing a language specifier.
📝 Suggested fix
-```
+```text
src/ui/order_form.rs # render_order_form, validation, preview📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #### Key files for codegen | |
| ``` | |
| src/ui/order_form.rs # render_order_form, validation, preview | |
| src/ui/currencies.rs # ISO list + resolve/filter helpers | |
| src/ui/orders.rs # FormState, FormField, CurrencyPicker, is_dirty | |
| src/ui/order_confirm.rs # render_order_confirm, render_leave_confirm | |
| src/ui/draw.rs # tab render + ConfirmLeaveOrder overlay | |
| src/ui/key_handler/form_input.rs # char/backspace + handle_currency_picker_key | |
| src/ui/key_handler/navigation.rs # tab leave guard, restore_or_new_form | |
| src/ui/key_handler/enter_handlers.rs # ConfirmingOrder / ConfirmLeaveOrder Enter | |
| src/ui/key_handler/esc_handlers.rs # Esc on form / leave guard | |
| ``` | |
| #### Key files for codegen | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 347-347: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for 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.
In `@docs/TUI_INTERFACE.md` around lines 345 - 357, Add a language specifier to
the fenced code block in the “Key files for codegen” section to satisfy MD040;
update the markdown fence around the file list in docs/TUI_INTERFACE.md to
include a language tag such as text, keeping the contents and surrounding
documentation unchanged.
Source: Linters/SAST tools
Contributor
There was a problem hiding this comment.
Oops, something went wrong! Please try again later. 🐰 💔
Comment on lines
+352
to
+452
| let amount_val = if form.amount.trim() == "0" || form.amount.trim().is_empty() { | ||
| Line::from(Span::styled( | ||
| "market", | ||
| Style::default() | ||
| .fg(Color::DarkGray) | ||
| .add_modifier(Modifier::ITALIC), | ||
| )) | ||
| } else { | ||
| Line::from(format!("{} sats", group_thousands(&form.amount))) | ||
| }; | ||
|
|
||
| let mut rows = vec![ | ||
| Row { | ||
| field: FormField::OrderType, | ||
| label: "Type", | ||
| value: type_val, | ||
| prefix_len: 0, | ||
| text_len: 0, | ||
| editable: false, | ||
| }, | ||
| Row { | ||
| field: FormField::Currency, | ||
| label: "Currency", | ||
| value: currency_val, | ||
| prefix_len: 0, | ||
| text_len: 0, | ||
| editable: false, | ||
| }, | ||
| Row { | ||
| field: FormField::AmountSats, | ||
| label: "Amount", | ||
| value: amount_val, | ||
| prefix_len: 0, | ||
| text_len: form.amount.len(), | ||
| editable: true, | ||
| }, | ||
| ]; | ||
|
|
||
| let (tag, tag_color) = if form.use_range { | ||
| ("[Range] ", Color::Magenta) | ||
| } else { | ||
| ("[Single] ", Color::Cyan) | ||
| }; | ||
| rows.push(Row { | ||
| field: FormField::FiatAmount, | ||
| label: if form.use_range { "Fiat min" } else { "Fiat" }, | ||
| value: Line::from(vec![ | ||
| Span::styled( | ||
| tag, | ||
| Style::default().fg(tag_color).add_modifier(Modifier::BOLD), | ||
| ), | ||
| Span::raw(&form.fiat_amount), | ||
| ]) | ||
| Span::raw(form.fiat_amount.clone()), | ||
| ]), | ||
| prefix_len: tag.chars().count(), | ||
| text_len: form.fiat_amount.len(), | ||
| editable: true, | ||
| }); | ||
|
|
||
| if form.use_range { | ||
| rows.push(Row { | ||
| field: FormField::FiatAmountMax, | ||
| label: "Fiat max", | ||
| value: Line::from(form.fiat_amount_max.clone()), | ||
| prefix_len: 0, | ||
| text_len: form.fiat_amount_max.len(), | ||
| editable: true, | ||
| }); | ||
| } | ||
|
|
||
| rows.push(Row { | ||
| field: FormField::PaymentMethod, | ||
| label: "Method", | ||
| value: dim_if_empty(&form.payment_method, "(any)"), | ||
| prefix_len: 0, | ||
| text_len: form.payment_method.len(), | ||
| editable: true, | ||
| }); | ||
| rows.push(Row { | ||
| field: FormField::Premium, | ||
| label: "Premium", | ||
| value: premium_line(&form.premium), | ||
| prefix_len: 0, | ||
| text_len: form.premium.len(), | ||
| editable: true, | ||
| }); | ||
| rows.push(Row { | ||
| field: FormField::Invoice, | ||
| label: "Invoice", | ||
| value: dim_if_empty(&form.invoice, "(optional)"), | ||
| prefix_len: 0, | ||
| text_len: form.invoice.len(), | ||
| editable: true, | ||
| }); | ||
| rows.push(Row { | ||
| field: FormField::ExpirationDays, | ||
| label: "Expiry", | ||
| value: expiry_line(&form.expiration_days), | ||
| prefix_len: 0, | ||
| text_len: form.expiration_days.len(), | ||
| editable: true, | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Cursor misaligns with the visible text for Amount, Premium, and Expiry fields.
Row.text_len uses the raw string length, but the rendered value for these three rows is decorated (thousands separators + " sats", "+N %", "N day(s)"/"no expiry"). The cursor (VALUE_OFFSET + prefix_len + text_len, set in render_details) will visually land inside the decorated text rather than at the true edit position — e.g. editing Premium with raw "12" renders "+12 %", but the cursor sits after only 2 characters into that string.
Editing itself still works (typing/backspace always operate on the raw string), but the visible caret position is misleading whenever these three fields are focused.
🐛 Suggested approach: show raw text while the field is focused
- let amount_val = if form.amount.trim() == "0" || form.amount.trim().is_empty() {
- Line::from(Span::styled(
- "market",
- Style::default()
- .fg(Color::DarkGray)
- .add_modifier(Modifier::ITALIC),
- ))
- } else {
- Line::from(format!("{} sats", group_thousands(&form.amount)))
- };
+ let amount_val = if form.focused == FormField::AmountSats {
+ Line::from(form.amount.clone())
+ } else if form.amount.trim() == "0" || form.amount.trim().is_empty() {
+ Line::from(Span::styled(
+ "market",
+ Style::default()
+ .fg(Color::DarkGray)
+ .add_modifier(Modifier::ITALIC),
+ ))
+ } else {
+ Line::from(format!("{} sats", group_thousands(&form.amount)))
+ };Apply the same form.focused == FormField::… gate to premium_line(&form.premium) and expiry_line(&form.expiration_days) call sites so text_len always matches what's on screen while editing.
🤖 Prompt for 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.
In `@src/ui/order_form.rs` around lines 352 - 452, The focused cursor position is
derived from Row.text_len in render_details, but Amount, Premium, and Expiry are
rendered with extra decoration from amount_val, premium_line, and expiry_line so
the caret no longer matches the visible text. Update the Row construction in
order_form.rs so these fields show the raw editable string whenever form.focused
is FormField::AmountSats, FormField::Premium, or FormField::ExpirationDays, and
keep the decorated display only when unfocused. This will make text_len align
with what is actually rendered and fix the cursor placement.
Contributor
There was a problem hiding this comment.
Oops, something went wrong! Please try again later. 🐰 💔
Contributor
There was a problem hiding this comment.
Oops, something went wrong! Please try again later. 🐰 💔
Comment on lines
+651
to
+732
| let max_rows = filtered.len().min(8) as u16; | ||
| let height = max_rows + 3; // border (2) + hint (1) | ||
| let mut y = anchor.y + 1; | ||
| if y + height > bounds.y + bounds.height { | ||
| y = anchor.y.saturating_sub(height); | ||
| } | ||
| let popup = Rect { | ||
| x, | ||
| y, | ||
| width, | ||
| height: height.min(bounds.y + bounds.height - y), | ||
| }; | ||
|
|
||
| f.render_widget(Clear, popup); | ||
|
|
||
| let title = if currencies.is_empty() { | ||
| format!(" Select currency ({} common) ", options.len()) | ||
| } else { | ||
| format!(" Select currency ({} accepted) ", options.len()) | ||
| }; | ||
| let block = Block::default() | ||
| .title(title) | ||
| .borders(Borders::ALL) | ||
| .border_type(BorderType::Rounded) | ||
| .style(Style::default().bg(BACKGROUND_COLOR).fg(PRIMARY_COLOR)); | ||
| let inner = block.inner(popup); | ||
| f.render_widget(block, popup); | ||
|
|
||
| let split = Layout::new( | ||
| Direction::Vertical, | ||
| [Constraint::Min(1), Constraint::Length(1)], | ||
| ) | ||
| .split(inner); | ||
|
|
||
| if filtered.is_empty() { | ||
| f.render_widget( | ||
| Paragraph::new(Span::styled( | ||
| " no match", | ||
| Style::default().fg(Color::DarkGray), | ||
| )) | ||
| .style(Style::default().bg(BACKGROUND_COLOR)), | ||
| split[0], | ||
| ); | ||
| } else { | ||
| let items: Vec<ListItem> = filtered | ||
| .iter() | ||
| .map(|o| { | ||
| let mut spans = vec![Span::styled( | ||
| format!("{:<5}", o.code), | ||
| Style::default().add_modifier(Modifier::BOLD), | ||
| )]; | ||
| if !o.name.is_empty() { | ||
| spans.push(Span::styled( | ||
| o.name.clone(), | ||
| Style::default().add_modifier(Modifier::DIM), | ||
| )); | ||
| } | ||
| ListItem::new(Line::from(spans)) | ||
| }) | ||
| .collect(); | ||
|
|
||
| let list = List::new(items) | ||
| .style(Style::default().fg(Color::White).bg(BACKGROUND_COLOR)) | ||
| .highlight_style( | ||
| Style::default() | ||
| .fg(Color::Black) | ||
| .bg(PRIMARY_COLOR) | ||
| .add_modifier(Modifier::BOLD), | ||
| ) | ||
| .highlight_symbol("› "); | ||
| let mut state = ListState::default().with_selected(Some(selected)); | ||
| f.render_stateful_widget(list, split[0], &mut state); | ||
|
|
||
| if filtered.len() > split[0].height as usize { | ||
| let mut sb_state = ScrollbarState::new(filtered.len()).position(selected); | ||
| f.render_stateful_widget( | ||
| Scrollbar::default().orientation(ScrollbarOrientation::VerticalRight), | ||
| split[0], | ||
| &mut sb_state, | ||
| ); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Empty-filter popup may starve the "no match" line of height.
height = max_rows + 3 (border 2 + hint 1) yields height = 3 when filtered is empty (max_rows = 0), leaving only 1 inner row for a [Min(1), Length(1)] split that needs 2 rows (no-match text + hint). The "no match" line can end up with 0 height depending on how the layout solver resolves the shortfall.
🩹 Suggested fix
- let max_rows = filtered.len().min(8) as u16;
- let height = max_rows + 3; // border (2) + hint (1)
+ let content_rows = filtered.len().min(8).max(1) as u16;
+ let height = content_rows + 3; // border (2) + hint (1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let max_rows = filtered.len().min(8) as u16; | |
| let height = max_rows + 3; // border (2) + hint (1) | |
| let mut y = anchor.y + 1; | |
| if y + height > bounds.y + bounds.height { | |
| y = anchor.y.saturating_sub(height); | |
| } | |
| let popup = Rect { | |
| x, | |
| y, | |
| width, | |
| height: height.min(bounds.y + bounds.height - y), | |
| }; | |
| f.render_widget(Clear, popup); | |
| let title = if currencies.is_empty() { | |
| format!(" Select currency ({} common) ", options.len()) | |
| } else { | |
| format!(" Select currency ({} accepted) ", options.len()) | |
| }; | |
| let block = Block::default() | |
| .title(title) | |
| .borders(Borders::ALL) | |
| .border_type(BorderType::Rounded) | |
| .style(Style::default().bg(BACKGROUND_COLOR).fg(PRIMARY_COLOR)); | |
| let inner = block.inner(popup); | |
| f.render_widget(block, popup); | |
| let split = Layout::new( | |
| Direction::Vertical, | |
| [Constraint::Min(1), Constraint::Length(1)], | |
| ) | |
| .split(inner); | |
| if filtered.is_empty() { | |
| f.render_widget( | |
| Paragraph::new(Span::styled( | |
| " no match", | |
| Style::default().fg(Color::DarkGray), | |
| )) | |
| .style(Style::default().bg(BACKGROUND_COLOR)), | |
| split[0], | |
| ); | |
| } else { | |
| let items: Vec<ListItem> = filtered | |
| .iter() | |
| .map(|o| { | |
| let mut spans = vec![Span::styled( | |
| format!("{:<5}", o.code), | |
| Style::default().add_modifier(Modifier::BOLD), | |
| )]; | |
| if !o.name.is_empty() { | |
| spans.push(Span::styled( | |
| o.name.clone(), | |
| Style::default().add_modifier(Modifier::DIM), | |
| )); | |
| } | |
| ListItem::new(Line::from(spans)) | |
| }) | |
| .collect(); | |
| let list = List::new(items) | |
| .style(Style::default().fg(Color::White).bg(BACKGROUND_COLOR)) | |
| .highlight_style( | |
| Style::default() | |
| .fg(Color::Black) | |
| .bg(PRIMARY_COLOR) | |
| .add_modifier(Modifier::BOLD), | |
| ) | |
| .highlight_symbol("› "); | |
| let mut state = ListState::default().with_selected(Some(selected)); | |
| f.render_stateful_widget(list, split[0], &mut state); | |
| if filtered.len() > split[0].height as usize { | |
| let mut sb_state = ScrollbarState::new(filtered.len()).position(selected); | |
| f.render_stateful_widget( | |
| Scrollbar::default().orientation(ScrollbarOrientation::VerticalRight), | |
| split[0], | |
| &mut sb_state, | |
| ); | |
| } | |
| } | |
| let content_rows = filtered.len().min(8).max(1) as u16; | |
| let height = content_rows + 3; // border (2) + hint (1) | |
| let mut y = anchor.y + 1; | |
| if y + height > bounds.y + bounds.height { | |
| y = anchor.y.saturating_sub(height); | |
| } | |
| let popup = Rect { | |
| x, | |
| y, | |
| width, | |
| height: height.min(bounds.y + bounds.height - y), | |
| }; | |
| f.render_widget(Clear, popup); | |
| let title = if currencies.is_empty() { | |
| format!(" Select currency ({} common) ", options.len()) | |
| } else { | |
| format!(" Select currency ({} accepted) ", options.len()) | |
| }; | |
| let block = Block::default() | |
| .title(title) | |
| .borders(Borders::ALL) | |
| .border_type(BorderType::Rounded) | |
| .style(Style::default().bg(BACKGROUND_COLOR).fg(PRIMARY_COLOR)); | |
| let inner = block.inner(popup); | |
| f.render_widget(block, popup); | |
| let split = Layout::new( | |
| Direction::Vertical, | |
| [Constraint::Min(1), Constraint::Length(1)], | |
| ) | |
| .split(inner); | |
| if filtered.is_empty() { | |
| f.render_widget( | |
| Paragraph::new(Span::styled( | |
| " no match", | |
| Style::default().fg(Color::DarkGray), | |
| )) | |
| .style(Style::default().bg(BACKGROUND_COLOR)), | |
| split[0], | |
| ); | |
| } else { | |
| let items: Vec<ListItem> = filtered | |
| .iter() | |
| .map(|o| { | |
| let mut spans = vec![Span::styled( | |
| format!("{:<5}", o.code), | |
| Style::default().add_modifier(Modifier::BOLD), | |
| )]; | |
| if !o.name.is_empty() { | |
| spans.push(Span::styled( | |
| o.name.clone(), | |
| Style::default().add_modifier(Modifier::DIM), | |
| )); | |
| } | |
| ListItem::new(Line::from(spans)) | |
| }) | |
| .collect(); | |
| let list = List::new(items) | |
| .style(Style::default().fg(Color::White).bg(BACKGROUND_COLOR)) | |
| .highlight_style( | |
| Style::default() | |
| .fg(Color::Black) | |
| .bg(PRIMARY_COLOR) | |
| .add_modifier(Modifier::BOLD), | |
| ) | |
| .highlight_symbol("› "); | |
| let mut state = ListState::default().with_selected(Some(selected)); | |
| f.render_stateful_widget(list, split[0], &mut state); | |
| if filtered.len() > split[0].height as usize { | |
| let mut sb_state = ScrollbarState::new(filtered.len()).position(selected); | |
| f.render_stateful_widget( | |
| Scrollbar::default().orientation(ScrollbarOrientation::VerticalRight), | |
| split[0], | |
| &mut sb_state, | |
| ); | |
| } | |
| } |
🤖 Prompt for 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.
In `@src/ui/order_form.rs` around lines 651 - 732, The popup sizing in the
currency selection render path can leave the empty-state message with no space
when filtered is empty. Update the sizing logic in the render block around the
popup Rect/Layout split so the inner area always reserves enough rows for both
the "no match" text and the hint, and make the height calculation in the
currency popup rendering robust against the zero-item case. Use the existing
symbols filtered, max_rows, popup, and the [Constraint::Min(1),
Constraint::Length(1)] split to locate the fix.
Contributor
There was a problem hiding this comment.
Oops, something went wrong! Please try again later. 🐰 💔
Show raw editable text for Amount, Premium, and Expiry while focused so the caret aligns with visible input. Reserve minimum height for the currency dropdown empty-filter state. Deduplicate blank/duplicate codes in resolve_options. Add MD040 language tag to TUI_INTERFACE codegen block. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
I did a pass on this PR and I do not currently see a blocker. What I specifically checked:
My read is that the behavioral changes are scoped in the right places. The most important correctness point here was the leave guard: it needs to protect against accidental tab navigation without trapping the user or silently dropping state. The current flow looks sound to me:
I also checked the new currency picker path. The instance-advertised accepted list / bundled fallback split looks reasonable, and I do not see it changing the underlying order field semantics beyond replacing fragile free-text entry with a controlled selection flow. I ran the local test suite on this branch as well, and it passed. So from my side this looks like a solid UI/UX improvement pass without an obvious correctness regression in the order-creation flow. |
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the current head. Overall this is a solid UX improvement.
One thing to watch: the currency picker now treats any alphanumeric key on the Currency row as picker input, so modified shortcuts like Ctrl+V are also swallowed there. If you want that row to stay shortcut-friendly, gate the picker-open path on unmodified typing only.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Redesigns the Create New Order tab for clearer layout, better use of space, and safer navigation away from in-progress drafts.
TRADE/PRICING/TERMS) with compact input strips, focus arrow (▸), inline ✓/✗ validation, and green section headers when completesrc/ui/currencies.rs): prefersMostroInstanceInfo.fiat_currencies_accepted, falls back to bundled ISO-4217 list; bold code + dim name (no emoji flags — poor terminal font support)ConfirmLeaveOrder): Left/Right tab navigation on a dirty form prompts Keep editing vs Leave; draft stored inAppState.order_form_draftand restored on return⇄ Space; tab auto-restores a usable form instead of a dead placeholderdocs/TUI_INTERFACE.mdanddocs/README.mdupdated for AI/contributor contextMotivation
The previous form left large empty areas, used free-text currency entry, and lost partial input when users switched tabs. This brings the tab in line with the rest of the TUI polish (My Trades, Messages) and instance-aware currency selection.
Breaking changes
None — order submission protocol and
FormStatefield semantics are unchanged.Checklist
cargo test— 157 tests)cargo fmt,cargo clippy --all-targets --all-features,cargo buildTest plan
SEPA Instant)Summary by CodeRabbit
New Features
Bug Fixes