From 2a9d32e78b697c24cfed055b3b8749c75f207de3 Mon Sep 17 00:00:00 2001 From: DanS Date: Sun, 12 Jul 2026 14:51:32 -0500 Subject: [PATCH] fix(market): close portfolio-editor gaps found in re-audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial re-audit of the group editor surfaced follow-ups to the prior hardening pass: - Dirty-check regression: the empty-currency->"USD" coercion was applied on save but not in the dirty comparison, so clearing the currency field left a group perpetually "dirty" (unsaved dot + Revert/Save never settling, a redundant persist on every Close). Both sides now share pfEditLabel() / pfEditCurrency() so they can't disagree — this also trims the label, so a trailing space no longer wedges the dirty state. - Whitespace-only labels (" ") passed validation and persisted as an invisible group; the trimmed label is now required by pfWorkingValid. - "Add entry" persists an empty placeholder up front; abandoning it (Close/ Esc/outside-click without adding an address) left a phantom "$0 · 0" group. The summary now prunes address-less groups while the editor is closed. - Manual price accepted inf/NaN (neither is < 0); sanitize with std::isfinite. - Escape closed the whole editor even when it was only meant to dismiss the open custom-color popup, discarding edits; guard on !IsPopupOpen. - HiDPI: scale the Z/T chip pill's vertical padding and the balance-breakdown baseline nudge that the prior pass missed. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/ui/windows/market_tab.cpp | 57 ++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/src/ui/windows/market_tab.cpp b/src/ui/windows/market_tab.cpp index 2e82025..73c3519 100644 --- a/src/ui/windows/market_tab.cpp +++ b/src/ui/windows/market_tab.cpp @@ -293,13 +293,29 @@ static void PortfolioBeginEdit(App* app, int index) // ---- Portfolio-editor persistence helpers (operate on the s_pfEdit working state) ---- // Build a PortfolioEntry from the working set, preserving off-form fields (grid geometry) from base. +// Normalized views of the two free-text buffers, applied identically by pfBuildWorking (what we store) +// and pfWorkingMatches (the dirty check) so the two can never disagree: a trailing space in the label +// or an emptied currency field must not leave the group perpetually "dirty" after a save. +static std::string pfEditLabel() // label with leading/trailing whitespace trimmed +{ + std::string s(s_pfEdit.label); + size_t i = 0, j = s.size(); + while (i < j && (unsigned char)s[i] <= ' ') i++; + while (j > i && (unsigned char)s[j - 1] <= ' ') j--; + return s.substr(i, j - i); +} +static std::string pfEditCurrency() // currency, defaulting an empty buffer to USD +{ + return s_pfEdit.manualCcy[0] != '\0' ? std::string(s_pfEdit.manualCcy) : std::string("USD"); +} + static config::Settings::PortfolioEntry pfBuildWorking(const config::Settings::PortfolioEntry& base) { config::Settings::PortfolioEntry e = base; - e.label = s_pfEdit.label; e.addresses = s_pfEdit.addrs; e.icon = s_pfEdit.icon; + e.label = pfEditLabel(); e.addresses = s_pfEdit.addrs; e.icon = s_pfEdit.icon; e.color = s_pfEdit.color; e.outlineOpacity = s_pfEdit.outlineOpacity; e.priceBasis = s_pfEdit.priceBasis; e.manualPrice = s_pfEdit.manualPrice; - e.manualCurrency = (s_pfEdit.manualCcy[0] != '\0') ? std::string(s_pfEdit.manualCcy) : std::string("USD"); + e.manualCurrency = pfEditCurrency(); e.showDrgx = s_pfEdit.showDrgx; e.showValue = s_pfEdit.showValue; e.show24h = s_pfEdit.show24h; e.showSparkline = s_pfEdit.showSparkline; e.sparklineInterval = s_pfEdit.sparkInterval; return e; @@ -308,10 +324,10 @@ static config::Settings::PortfolioEntry pfBuildWorking(const config::Settings::P // True when the working set matches a stored entry (i.e. there are no uncommitted edits). static bool pfWorkingMatches(const config::Settings::PortfolioEntry& e) { - return e.label == std::string(s_pfEdit.label) && e.addresses == s_pfEdit.addrs && e.icon == s_pfEdit.icon + return e.label == pfEditLabel() && e.addresses == s_pfEdit.addrs && e.icon == s_pfEdit.icon && e.color == s_pfEdit.color && e.outlineOpacity == s_pfEdit.outlineOpacity && e.priceBasis == s_pfEdit.priceBasis && e.manualPrice == s_pfEdit.manualPrice - && e.manualCurrency == std::string(s_pfEdit.manualCcy) + && e.manualCurrency == pfEditCurrency() && e.showDrgx == s_pfEdit.showDrgx && e.showValue == s_pfEdit.showValue && e.show24h == s_pfEdit.show24h && e.showSparkline == s_pfEdit.showSparkline && e.sparklineInterval == s_pfEdit.sparkInterval; } @@ -321,7 +337,7 @@ static bool pfWorkingMatches(const config::Settings::PortfolioEntry& e) // button and the commit path (Close/switch commit too, so validating only the button wouldn't be enough). static bool pfWorkingValid() { - return s_pfEdit.label[0] != '\0' + return !pfEditLabel().empty() // a whitespace-only label is treated as empty (invisible group) && !s_pfEdit.addrs.empty() && !(s_pfEdit.priceBasis == 3 && s_pfEdit.manualPrice <= 0.0); } @@ -524,7 +540,8 @@ static void pfDrawPriceSection() float half = (ImGui::GetContentRegionAvail().x - Layout::spacingSm()) * 0.62f; ImGui::SetNextItemWidth(half); ImGui::InputDouble("##pfManPrice", &s_pfEdit.manualPrice, 0.0, 0.0, "%.6f"); - if (s_pfEdit.manualPrice < 0.0) s_pfEdit.manualPrice = 0.0; + // Reject negatives and non-finite input (InputDouble parses "inf"/"nan"; neither is < 0). + if (!std::isfinite(s_pfEdit.manualPrice) || s_pfEdit.manualPrice < 0.0) s_pfEdit.manualPrice = 0.0; ImGui::SameLine(); ImGui::SetNextItemWidth(-1); ImGui::InputTextWithHint("##pfManCcy", TR("portfolio_currency"), s_pfEdit.manualCcy, sizeof(s_pfEdit.manualCcy)); @@ -667,7 +684,7 @@ static void pfDrawAddressSection(App* app) ImU32 chipCol = isZ ? Success() : Warning(); ImVec2 chipPos(rx, rcy - capF->LegacySize * 0.5f - 2.0f * dp); float chipW = material::DrawPill(ldl, chipPos, chip, capF, chipCol, - WithAlpha(chipCol, 40), 0, ImVec2(4.0f * dp, 2.0f), 4.0f * dp).x; + WithAlpha(chipCol, 40), 0, ImVec2(4.0f * dp, 2.0f * dp), 4.0f * dp).x; rx += chipW + Layout::spacingSm(); std::string aicon = app->getAddressIcon(a.address); if (!aicon.empty()) { @@ -728,7 +745,11 @@ static void RenderPortfolioEditor(App* app) ov.cardHeight = 760.0f; ov.idSuffix = "pf"; if (!material::BeginOverlayDialog(ov)) return; - if (ImGui::IsKeyPressed(ImGuiKey_Escape)) s_pfEdit.open = false; + // A first Escape should dismiss only an open popup (e.g. the custom-color picker); don't also tear + // down the whole editor — which discards uncommitted edits — while a popup is capturing the key. + if (ImGui::IsKeyPressed(ImGuiKey_Escape) && + !ImGui::IsPopupOpen("", ImGuiPopupFlags_AnyPopupId | ImGuiPopupFlags_AnyPopupLevel)) + s_pfEdit.open = false; { // Clamp/validate the selection. The in-editor delete already reloads the working state, but if the // list shrank via any other path, reload after clamping so a later commit can't overlay stale @@ -748,8 +769,8 @@ static void RenderPortfolioEditor(App* app) float detailW = contentW - masterW - gap; // Does the selected group have uncommitted edits? (Only the selected group can differ from the - // stored copy — others were committed on deselect.) Drives the master-list "unsaved" dot and the - // detail-pane Revert/Save enable state. + // stored copy — the working buffer only ever holds the selected group; switching reloads it and + // discards any uncommitted edits.) Drives the master-list "unsaved" dot and Revert/Save state. bool selDirty = false; { const auto& es = settings->getPortfolioEntries(); @@ -1770,6 +1791,20 @@ static void mktDrawPortfolio(const MktCtx& cx) float ratioBarH = cx.ratioBarH, pfSummaryH = cx.pfSummaryH, portfolioH = cx.portfolioH; char buf[128]; + // Garbage-collect abandoned placeholders. "Add entry" persists an empty group up front so it shows + // in the editor list; if the user closes the editor without ever adding an address, that phantom + // "$0 · 0" group would linger. A group with no addresses can't be Saved (the editor forbids it), so + // it is always junk — prune it, but only while the editor is closed so we never delete one the user + // is actively filling in. + if (!s_pfEdit.open) { + auto es = app->settings()->getPortfolioEntries(); + size_t before = es.size(); + es.erase(std::remove_if(es.begin(), es.end(), + [](const config::Settings::PortfolioEntry& e) { return e.addresses.empty(); }), + es.end()); + if (es.size() != before) pfPersist(app->settings(), es); + } + Type().textColored(TypeStyle::Overline, OnSurfaceMedium(), TR("market_portfolio")); // "Manage…" button, right-aligned on the header row — opens the portfolio editor. { @@ -1834,7 +1869,7 @@ static void mktDrawPortfolio(const MktCtx& cx) snprintf(buf, sizeof(buf), "Z %.4f \xC2\xB7 T %.4f", private_balance, transparent_balance); float brkW = capFont->CalcTextSizeA(capFont->LegacySize, FLT_MAX, 0, buf).x; dl->AddText(capFont, capFont->LegacySize, - ImVec2(rightEdge - brkW, cy + 2), OnSurfaceDisabled(), buf); + ImVec2(rightEdge - brkW, cy + 2.0f * mktDp), OnSurfaceDisabled(), buf); cy += body2->LegacySize + Layout::spacingSm(); // Full-width shielded/transparent ratio bar + % label.