Skip to content

Improve p2p-transfer UI (#38)#41

Merged
radumarias merged 11 commits into
radumarias:mainfrom
StefBisti:improve-ui-38
May 15, 2026
Merged

Improve p2p-transfer UI (#38)#41
radumarias merged 11 commits into
radumarias:mainfrom
StefBisti:improve-ui-38

Conversation

@StefBisti
Copy link
Copy Markdown
Contributor

@StefBisti StefBisti commented May 6, 2026

Summary

  • Redesigns the entire visual presentation of p2p-transfer using generated dark design system
  • No functionality was changed — only the UI appearance

Changes

  • Header: Single slim 54px bar replacing the old double top-panel; includes mode label, quick-access buttons, and theme toggle
  • Home screen: Two side-by-side mode cards (Send / Receive) shown when the app is idle — replaces the old empty "No file selected" state
  • Cards: File info, shared files queue, active share URL, and received files all styled as proper cards with borders, rounded corners, and icon boxes
  • Share URL: Prominently displayed in a highlighted card with a green pulsing dot, monospace URL box, and "Copy Link" button
  • Receive panel: Clean card with labeled inputs and inline connection status
  • Terminal: Collapsed to a slim 44px footer bar showing only the latest log entry

Assisted by Stitch and Claude Code

Screenshot 2026-05-06 at 21 20 30 Screenshot 2026-05-06 at 21 24 28 Screenshot 2026-05-06 at 21 24 41

StefBisti and others added 3 commits May 6, 2026 20:26
Redesigns the entire UI appearance of p2p-transfer using the dark color
palette and layout from the Stitch-generated mockups. No functionality
was changed — only visual presentation.

Key changes:
- Apply Stitch dark color palette (navy bg, indigo primary, emerald secondary)
- Replace double top-panel with a single slim 54px header bar
- Add home screen with two mode cards (Send / Receive) shown when idle
- Style file info, shared files, and connection status as proper cards
- Make the shareable URL prominent with a Copy Link button and green status dot
- Extract receive panel into a clean card with labeled fields
- Collapse terminal to a slim 44px footer bar (latest log only)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@StefBisti StefBisti marked this pull request as ready for review May 6, 2026 18:19
@StefBisti StefBisti requested a review from radumarias as a code owner May 6, 2026 18:19
@radumarias
Copy link
Copy Markdown
Owner

Code review

Found 3 issues:

  1. "Receive" button text uses a hardcoded literal that only matches the dark theme (bug due to Button::new(RichText::new("Receive").color(Color32::from_rgb(0, 56, 36))...).fill(tc.secondary) in show_home_cards). The literal (0, 56, 36) equals Tc::dark().on_secondary but Tc::light().on_secondary is (255, 255, 255). In light mode tc.secondary is (0, 108, 74), so the label renders as very-dark green on dark green and is effectively unreadable. The sibling Send button correctly uses tc.on_primary — this one should use tc.on_secondary.

ui.add_space(20.0);
if ui.add(
Button::new(RichText::new("Receive").color(Color32::from_rgb(0, 56, 36)).strong().size(15.0))
.fill(tc.secondary)
.rounding(CornerRadius::same(8))
.min_size(Vec2::new(150.0, 38.0))
).clicked() {
open_receive = true;
}
});

  1. Previously received files become invisible after returning to the home screen (bug due to the new central-panel dispatcher in update()). The old code called self.show_received_files(ui) unconditionally at the bottom of the central panel; the new code only calls it inside the else branch and inside show_receive_panel. The Cancel / Stop paths in the receive flow set show_receive_dialog = false without clearing self.received_files, so a user who completes a transfer and then closes the receive panel lands on show_home_cards (because !has_file && !has_shared && !is_accepting), which never renders the received-files list — the files remain in state but disappear from the UI.

if !has_file && !has_shared && !self.is_accepting {
self.show_home_cards(ui, ctx);
} else {
self.show_file_info(ui);
self.show_shared_files(ui, ctx);
self.show_connection_status(ui);
self.show_received_files(ui);
}
});

  1. Status-color heuristic miscolors real failures as success (bug due to let status_color = if status.starts_with("Error") { tc.error } else { tc.secondary }; in show_receive_panel). Several real failure messages produced elsewhere in this file do NOT start with "Error" — e.g. extract_node_id returns Err("Invalid node ID or URL format") which the Connect handler writes verbatim into receive_status, and the node-spawn path writes format!("Connection failed: {}", e). Both render in tc.secondary (the success green), so a failed connection looks like a successful state.

if let Ok(status) = self.receive_status.lock() {
if !status.is_empty() {
let status_color = if status.starts_with("Error") { tc.error } else { tc.secondary };
ui.label(RichText::new(status.as_str()).color(status_color).size(13.0));
ui.add_space(8.0);

@radumarias
Copy link
Copy Markdown
Owner

Code review (full)

Findings from /review. The three highest-signal items from this list were also posted as a separate auto code review above; this post includes the full set with strengths, UX notes, and convention nits. Specific lines are flagged inline below.

Overview

Pure UI/visual refactor of p2p-transfer/src/app.rs (+698 / -419). Introduces a "Stitch" dark+light token palette (Tc), apply_theme to bind it to egui Visuals, a slim 54px header, a home screen with side-by-side Send/Receive cards, card-styled file/shared/received sections, and a collapsing terminal footer (44px collapsed / 200px expanded). No transport, protocol, or state machine changes.

Strengths

  • Centralized palette (Tc::dark() / Tc::light()) is a clean abstraction over the previous scattered Color32::from_rgb(...) literals.
  • The card primitive (egui::Frame + CornerRadius + Stroke) is reused consistently across sections — nice visual coherence.
  • The collapsing terminal panel is a clear UX win over the previous fixed screen_rect().height()/3.0 footer.
  • Header mode label (SEND / RECEIVE / SHARING) gives useful state visibility.

Bugs & correctness

  1. Hardcoded text color breaks the light themeColor32::from_rgb(0, 56, 36) on the home "Receive" button text matches Tc::dark().on_secondary but not Tc::light().on_secondary (white). In light mode the button renders very-dark-green on dark-green and is unreadable. Should use tc.on_secondary.
  2. Tinted overlays use dark-mode-only RGB literals — the active-share-card border and the two home-card icon halos use literal (78, 222, 163, …) / (192, 193, 255, …) values, which are the dark-palette primary/secondary base colors. In light mode tc.primary is #4143c7 and tc.secondary is #006c4a, so the tinted accents don't match the active theme. Replace with let c = tc.primary; Color32::from_rgba_unmultiplied(c.r(), c.g(), c.b(), 30).
  3. apply_theme runs every frameSelf::apply_theme(ctx) at the top of update() calls ctx.set_visuals(v) unconditionally, forcing a style rebuild every frame. Cache the last dark_mode on &mut self and only call when it flips.
  4. Stray debug log preserved in render pathweb_sys::console::log_1(&format!("============{:?}", size).into()) was moved into the new show_file_info body; it fires every frame in WASM. Remove it.
  5. Status-color heuristic miscolors real failuresstatus.starts_with("Error") colors only strings beginning with "Error" as error red; the rest render in success-green. Real failure paths produce "Connection failed: …" and "Invalid node ID or URL format" — both flagged success. See the dedicated auto code review above.
  6. Home-screen branch hides previously received filesshow_home_cards does not render show_received_files; combined with the receive panel's Cancel/Stop paths that don't clear received_files, the list silently disappears after a transfer. See the dedicated auto code review above.
  7. Missing trailing newline at EOF\ No newline at end of file in the diff. cargo fmt --check (part of ./check.sh) will fail. Add it.

UX issues

  • No path back to the Home screen. Once a file is picked or shares exist, the dispatcher routes away from show_home_cards and there is no UI affordance to clear state. Consider a "home" / clear action on the file card.
  • Dead-ish magnet_input UI. show_file_info still renders a Copy box for self.magnet_input, but the field is only initialized to String::new() in this diff and I don't see it being written anywhere. Either wire it up or delete the block.

Convention / minor nits

  • share_btn.clone().on_hover_text(...) discards the response; works in egui but is confusing. share_btn.on_hover_text("…").clicked() is cleaner.
  • Triple-locking per frame: update() locks picked_file_name / shared_files to pick a branch, then show_file_info / show_shared_files re-lock them. Cheap under no contention; compute once and pass down.
  • The Tc color table uses tab-aligned padding (bg: Color32::from_rgb(...)); cargo fmt will rewrite it on first CI run.

Pre-existing (not introduced by this PR, but visible)

  • show_received_files is gated #[cfg(not(target_arch = "wasm32"))] but called from the shared update() path (and now also from show_receive_panel). Either the gate is wrong or the call sites need matching cfg. Worth a separate cleanup.

Risk

Visual-only PR; low blast radius. The theme/contrast bugs are the only items that change observable behavior across themes — the rest is cosmetic. Run ./check.sh (CI gate per p2p-transfer/CLAUDE.md) before merge — the missing trailing newline will trip cargo fmt --check.

Copy link
Copy Markdown
Owner

@radumarias radumarias left a comment

Choose a reason for hiding this comment

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

Inline notes from /review. See the full writeup in the issue comment above.

Comment thread p2p-transfer/src/app.rs Outdated
ui.label(RichText::new("Paste a share link to download directly").color(tc.on_surface_var).size(14.0));
ui.add_space(20.0);
if ui.add(
Button::new(RichText::new("Receive").color(Color32::from_rgb(0, 56, 36)).strong().size(15.0))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hardcoded Color32::from_rgb(0, 56, 36) matches Tc::dark().on_secondary but Tc::light().on_secondary is (255, 255, 255). With a light-mode tc.secondary background of (0, 108, 74) (dark green), this text becomes unreadable dark-green-on-green. Use tc.on_secondary (the sibling Send button correctly uses tc.on_primary).

Comment thread p2p-transfer/src/app.rs Outdated
ui.label("Link/Hash:");
ui.text_edit_singleline(&mut self.receive_hash_input);
});
Self::apply_theme(ctx);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Self::apply_theme(ctx) runs every frame and calls ctx.set_visuals(v) unconditionally, triggering an egui style rebuild at the frame rate. Track theme state on &mut self (or compare ctx.style().visuals.dark_mode with a cached value) and only call when it actually flips.

Comment thread p2p-transfer/src/app.rs Outdated
ui.group(|ui| {
let size_str = self.format_size(size);
#[cfg(target_arch = "wasm32")]
web_sys::console::log_1(&format!("============{:?}", size).into());
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Stray debug print — fires every frame in the WASM build whenever show_file_info renders. Drop it.

Comment thread p2p-transfer/src/app.rs Outdated
let card = egui::Frame::new()
.fill(tc.surface_low)
.rounding(CornerRadius::same(10))
.stroke(Stroke::new(1.0, Color32::from_rgba_unmultiplied(78, 222, 163, 60)))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Tinted border RGB (78, 222, 163, 60) is the dark-mode secondary base color with alpha. In light mode tc.secondary is #006c4a, so the border doesn't match the active theme. Use let c = tc.secondary; Color32::from_rgba_unmultiplied(c.r(), c.g(), c.b(), 60).

Comment thread p2p-transfer/src/app.rs Outdated
ui.set_min_height(200.0);
ui.vertical_centered(|ui| {
let (ico_rect, _) = ui.allocate_exact_size(Vec2::splat(56.0), egui::Sense::hover());
ui.painter().circle_filled(ico_rect.center(), 28.0, Color32::from_rgba_unmultiplied(192, 193, 255, 30));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Same theme-mismatch as the active-share border: (192, 193, 255, 30) is dark-mode primary with alpha. In light mode tc.primary is #4143c7, so this halo doesn't follow the theme. Derive from tc.primary channels instead of a literal.

Comment thread p2p-transfer/src/app.rs Outdated
ui.label("Easily share files with a secure peer-to-peer connection");
ui.add_space(5.0);
let (ico_rect, _) = ui.allocate_exact_size(Vec2::splat(56.0), egui::Sense::hover());
ui.painter().circle_filled(ico_rect.center(), 28.0, Color32::from_rgba_unmultiplied(78, 222, 163, 30));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Same as the Send icon halo — (78, 222, 163, 30) is dark-mode secondary. Derive from tc.secondary channels so the halo follows the active theme.

Comment thread p2p-transfer/src/app.rs Outdated
ui.add_space(5.0);

if ui.button("📋 Copy").clicked() {
if !self.magnet_input.is_empty() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

magnet_input is initialized to String::new() in Default::default() and I can't find any writer in this file. If this Copy URI box is dead UI, delete it; otherwise leave a TODO referencing the intended wiring.

Comment thread p2p-transfer/src/app.rs Outdated
);
share_btn.clone().on_hover_text("Start accepting connections and share all files");

share_btn.clone().on_hover_text("Start sharing all files");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

share_btn.clone().on_hover_text(...) discards the cloned response. The tooltip still registers via the response id so it works, but share_btn.on_hover_text("…").clicked() reads cleaner.

@StefBisti StefBisti requested a review from radumarias May 13, 2026 13:25
…F newline

- Drop stale `#[cfg(not(target_arch = "wasm32"))]` on `show_received_files`. The body is pure egui and `ReceivedFile` is platform-agnostic; the gate caused `cargo check --target wasm32-unknown-unknown` to fail because the function is called unconditionally from the central dispatcher (and from `show_receive_panel` after c03b04c).
- Add a "🏠 Home" button in the header (shown only when not already on the idle home screen) that exits the receive dialog and reuses `stop_accepting()` to clear node, share URL, shared files, and picked-file state. Closes the "no path back to home" gap flagged in the /review.
- Add the missing trailing newline at EOF so `cargo fmt --check` no longer reports `\ No newline at end of file` on this file.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@radumarias radumarias merged commit 0c97217 into radumarias:main May 15, 2026
0 of 3 checks passed
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.

2 participants