refactor(ui): migrate Panel to show_inside(ui, …) and remove all egui 0.34 deprecation suppressions#858
Draft
lklimek wants to merge 5 commits into
Draft
Conversation
Rename `default_width`/`min_width`/`max_width` to `default_size`/`min_size`/`max_size` in the 9 panel helpers per egui 0.34's axis-agnostic API. Trait-stable, no plumbing changes; first step of the show_inside migration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…s to show_inside Flip the ScreenLike::ui trait signature from &Context to &mut egui::Ui, propagate through the Screen enum dispatch and all 60+ screen impls, and migrate panel helpers (island_central_panel, add_top_panel, add_left_panel ×2, the four subscreen choosers, add_contract_chooser_panel) from Panel::show(ctx, …) to Panel::show_inside(ui, …). Squashed because the helper signature flip leaves the trait-only commit with ~50 unused egui::Context imports across screen files, which fail cargo clippy --all-features --all-targets -- -D warnings. The two changes are semantically one refactor — keeping every commit clippy-clean matters for bisect and per-commit CI gates. Outer-frame dark_mode reads now go through ui.ctx().global_style() to preserve QA-002 discipline (global, not local). The 17 remaining #[allow(deprecated)] sites are direct screen-level CentralPanel::show calls — handled in the next commit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migrate the 17 direct `egui::CentralPanel::default()…show(ctx, …)` sites in screens to `show_inside(ui, …)` and drop the surrounding `#[allow(deprecated)]`. With the trait now passing `&mut Ui` from `App::ui` (TASK-2) and the helpers using `show_inside` (TASK-3), every screen-level panel can do the same. The two `egui::Window::show(ctx, …)` and `WalletUnlockPopup::show(ctx, …)` call sites adjacent to these panels are unaffected — `Window::show` and the custom popup method are not deprecated, so they keep their original shape (using `ui.ctx()` where `ctx` was previously bound). After this commit, `src/ui/` and `src/app.rs` contain zero `#[allow(deprecated)]` annotations. The 6 AES-ECB / DIP-0015 suppressions in `src/model/wallet/` and `src/backend_task/dashpay/` remain untouched. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Final cleanup after the egui 0.34 `show_inside` migration. The `TODO(egui-0.35)` breadcrumb on `island_central_panel` was already removed while flipping its signature in TASK-3 — this commit captures the residual `cargo +nightly fmt` adjustments to import grouping and call wrapping. All 27 in-scope `#[allow(deprecated)]` annotations are gone; the 6 AES-ECB suppressions tied to DIP-0015 in `src/model/wallet/` and `src/backend_task/dashpay/` remain untouched and intentional. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Screen Pattern section documented the pre-egui-0.34 trait shape (`ui(&mut self, ctx: &Context) -> AppAction`). After the show_inside migration the trait takes `&mut egui::Ui` directly. Update the bullet to match the actual signature in src/ui/mod.rs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
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
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
Follow-up to #857 (egui 0.33.3 → 0.34.2 upgrade). Removes the 27
#[allow(deprecated)]suppressions left by the upgrade PR's containment strategy by performing the structural migration deferred there:ScreenLike::uitrait signature from&Contextto&mut egui::Ui(60+ screens, mechanical)island_central_panel,add_top_panel,add_left_panel,add_left_wallet_panel, plus the 5 chooser panels) fromPanel::show(ctx, …)toPanel::show_inside(ui, …)egui::CentralPanel::default().show(ctx, …)sites in screens toshow_inside(ui, …)default_width→default_size,min_width/max_width→min_size/max_size)TODO(egui-0.35)breadcrumb planted by the upgrade PRCLAUDE.md"Screen Pattern" section to document the new&mut UisignatureAfter this PR, zero egui-related
#[allow(deprecated)]suppressions remain anywhere in the codebase. The 6 AES-ECB / DIP-0015 suppressions insrc/model/wallet/andsrc/backend_task/dashpay/are intentional cryptographic protocol choices (unrelated to egui) and remain untouched.Why "Option A" wasn't viable
The upgrade PR's optimistic plan was to leave the trait alone and only flip panel helpers to take
&mut Ui. That doesn't work: panel helpers are invoked from insideScreen::ui(ctx)bodies (not fromApp::uidirectly), so the only&Contextavailable at the helper boundary is the screen's own. Wrapping each screen in a syntheticCentralPanel::show(ctx, …)adapter to obtain aUiwould re-introduce the same deprecated call we're trying to remove. The trait flip is the only clean path.Commit decomposition (4 refactor + 1 doc)
refactor(ui): rename Panel axis-agnostic methods (default/min/max_size)— trait-stable, removes one class of deprecationrefactor(ui): flip ScreenLike::ui to &mut Ui and migrate panel helpers to show_inside— semantically one refactor (squashed because the trait flip leaves the standalone commit with unusedContextimports that the helper signature flip cleans up)refactor(ui): migrate screen central panels to show_inside(ui, …)— last 17 deprecation sitesrefactor(ui): drop egui-0.35 TODO breadcrumb after panel migrationdocs: update ScreenLike::ui signature in CLAUDE.md after trait flipEvery commit compiles cleanly and passes
cargo clippy --all-features --all-targets -- -D warnings.Stack
This PR targets
chore/upgrade-egui-0.34(#857). It should be reviewed and merged after #857 lands; GitHub will auto-rebase the base.User-visible impact
None. Pure internal refactor — no UX changes, no functional changes, no string changes (no i18n surface touched).
Test plan
cargo build --all-featurescleancargo +nightly fmt --all -- --checkcleancargo clippy --all-features --all-targets -- -D warningsclean — at every commit (verified by checking out each intermediate SHA, not just HEAD)cargo test --all-features --workspace— 553 tests pass (468 unit + 10 e2e + 72 kittest + 3 doctests; ignored: 2 unit + 14 doctests + 59 backend-e2e)cargo test --test kittest --all-features— 72/72 pass (matches PR chore(deps): upgrade egui to 0.34.2 #857 baseline)cargo run, navigate the seven root screens (Identities, DPNS, Wallets, Tools, Contracts, Tokens, Settings), open at least one info popup per dashpay subscreen. Watch for unexpected double-frames, vanished margins, or banner placement drift. The migration is layout-equivalent in egui's own model, but verify visually.Notes for reviewers
ScreenLike::ui(&mut self, ui: &mut Ui)is an internal API. No external callers.ctx.global_style()notui.style(): where helpers needed global theme (e.g. outer-frame coloring), the substitution isui.ctx().global_style()— preserving global semantics.ui.style()(local cumulative style) was deliberately not used in those positions; see PR chore(deps): upgrade egui to 0.34.2 #857 QA-002.Context::style()itself deprecated: in egui 0.34 — Bilby substitutedui.ctx().global_style()rather thanui.ctx().style()to avoid re-introducing a deprecation. Documented in commit 2.tests/backend-e2e/errors (unresolvedwallets()/db()/sdk()accessors). These are gated by#[cfg(any(test, feature = "testing"))]insrc/context/mod.rs:862; rust-analyzer doesn't enable--features testingby default.cargo build --all-featuresandcargo test --all-features --workspaceresolve them. Configure rust-analyzer with"rust-analyzer.cargo.features": "all"if the noise bothers you.🤖 Co-authored by Claudius the Magnificent AI Agent