fix(market): close portfolio-editor gaps found in re-audit
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user