Skip to content

fix: synchronize SummaryView page subscriptions#630

Open
hcoona wants to merge 1 commit into
Yafc-CE:masterfrom
hcoona:dev/shuaizhang/fix-summary-view-project-subscriptions
Open

fix: synchronize SummaryView page subscriptions#630
hcoona wants to merge 1 commit into
Yafc-CE:masterfrom
hcoona:dev/shuaizhang/fix-summary-view-project-subscriptions

Conversation

@hcoona
Copy link
Copy Markdown

@hcoona hcoona commented May 9, 2026

Summary

Keep SummaryView subscriptions aligned with the current project and its current pages.

Detailed explanation

SummaryView subscribed to the project pages that existed at the time it was initialized or rebound. It did not reliably synchronize subscriptions when pages were added or removed, and project switching could leave the view subscribed to pages from the old project.

Example UI steps that trigger the issue

  1. Open a project and open the Summary view.
  2. Add or remove production pages, or switch to a different project.
  3. Modify the contents of the current project pages and observe the Summary view.

Expected behavior

The Summary view should subscribe only to pages in the current project, update when current pages change, and release subscriptions for deleted pages or previous projects.

Actual behavior

The Summary view can miss updates for newly added pages or keep stale subscriptions to deleted pages or a previous project.

Fix

SummaryView now tracks the set of subscribed pages explicitly. It synchronizes page subscriptions when project metadata changes, removes subscriptions for pages that are no longer present, subscribes to newly added pages, and rebinds cleanly when the active project changes.

@hcoona hcoona requested a review from DaleStan as a code owner May 9, 2026 22:08
Copy link
Copy Markdown
Collaborator

@veger veger left a comment

Choose a reason for hiding this comment

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

Thank you for this contribution. I have one remark on your PR.

scrollArea.Build(gui);
}

protected override void ModelContentsChanged(bool visualOnly) => Recalculate(visualOnly);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What does this override fix? Summary only has showOnlyIssues, and it's a plain auto-property with no RecordUndo() call, so changing it doesn't fire ContentChanged. The checkbox handler at line 265 already calls Recalculate() right after the toggle, and SetProject keeps
allGoods up to date anyway.

The only path I can find where this actually changes behavior is when SetModel calls ModelContentsChanged(false) as the user opens the Summary tab. Before, that was just a UI redraw; now it does a full recompute every time the tab is opened.

If you're adding it defensively to mirror ProductionSummaryView, could you add a short comment explaining what it's guarding against? Otherwise I think it can be dropped, which would keep this PR focused on the subscription fix.

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