Repository navigation
fix: show findings once and refresh the open rule detail after a Rule Editor save - #378
Open
Abhiram Shaji (abhiramshaji-cyber) wants to merge 2 commits into
Conversation
… Editor save Resolves microsoft#352
github-actions
Bot
requested review from
TamasBoncz,
Aymen (aymenfurter) and
Sanjay Singh (san360)
November 14, 2026 03:07
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.
Description
Saving a rule made the findings list repeat every practice group and left the open rule detail out of date, and this PR shows each group once plus the saved rule.
Hi there, hope you're all doing well! Thanks a lot Sanjay Singh (@san360) for such a clear report. The line numbers and the 4, 8, 12 count from your probe made this really quick to track down. I'd love any feedback on whether this is the shape you want.
What this code is for
renderAntiPatternsinsrc/webview/page-antipatterns.tsdraws the whole page into one container: the score cards, the findings under#apDetails, and the Rules tab with its rule detail panel. It runs when the page is opened, and again into the same container after a Rule Editor save, through the callback passed towireRuleEditorModal.Before this PR
A user opens Rules, clicks a rule, clicks Edit Source, then Save.
saveRuleanswers and the editor callsrenderAntiPatterns(container, currentFilter)again.#apDetailselement, with the sections added last time still inside it.renderFindingsbuilds 4 new<details>sections and adds each withdetailsContainer.appendChild(section)(this line on main).#apDetailsnow holds 8 sections. A second save makes it 12.#rule-detail-panelstays visible because onlyshowRuleDetailwrites its content, and nothing calls it again, so the detail still shows the rule from before the save.After this PR
Same user, same Save.
renderFindingscollects the 4 sections in an array and callsdetailsContainer.replaceChildren(...sections), so whatever was in#apDetailsis dropped.#apDetailsholds 4 sections after every save.showRuleDetailnow records the shown rule inpanel.dataset.ruleId. At the end ofrenderAntiPatterns, if that is set, the page callsshowRuleDetailagain with the rules it just fetched, so the detail shows the saved values.showRuleDetailcallshideRuleDetailand the user is back on the rule list.flowchart LR classDef box fill:#eef1ff,stroke:#4c5bd4,color:#111827 classDef bad fill:#ffd6d6,stroke:#cc0000,color:#111827,stroke-width:3px classDef good fill:#c8f0c8,stroke:#008800,color:#111827,stroke-width:3px subgraph BEFORE["BEFORE: 4, then 8, then 12 sections"] direction TB B1["Save in the Rule Editor"] --> B2["renderAntiPatterns"] B2 --> B3["renderFindings builds 4 sections"] B3 --> B4{"appendChild into apDetails"} B4 -->|"old sections kept"| B5["8 sections, detail shows old values"] B4 -->|"first open of the page"| B6["4 sections"] end subgraph AFTER["AFTER: 4 sections on every save"] direction TB A1["Save in the Rule Editor"] --> A2["renderAntiPatterns"] A2 --> A3["renderFindings builds 4 sections"] A3 --> A4{"THE CHANGE: replaceChildren on apDetails"} A4 -->|"every save"| A5["4 sections, detail shows the saved rule"] A4 -->|"first open of the page"| A6["4 sections, same as today"] end B1 ~~~ A1 class B1,B2,B3,B6 box class A1,A2,A3,A6 box class B4,B5 bad class A4,A5 good linkStyle 3 stroke:#cc0000,stroke-width:5px linkStyle 8 stroke:#008800,stroke-width:5px style BEFORE fill:#fff5f5,stroke:#cc0000,color:#111827 style AFTER fill:#f3fff3,stroke:#008800,color:#111827renderFindingsonmain(left) and on this branch (right).The change
renderFindingsends withdetailsContainer.replaceChildren(...sections)instead of callingappendChildper section. It always builds all 4 groups, including ones with no findings, so this covers the first open, every save, and empty data the same way.showRuleDetailsetspanel.dataset.ruleId, the Back button and a missing rule both go through a newhideRuleDetailthat clears it, andrenderAntiPatternsshows the recorded rule again after it renders.renderPage, so nothing changes there.What changes for a user
One question: for the open detail I went with showing the saved rule again rather than closing it, since the issue mentions both. Would you prefer it to close instead? Happy to switch it.
Related Issues
Fixes #352
Checklist
npm run checkpasses (typecheck + lint + spellcheck + knip + tests). Typecheck, lint, spellcheck, knip and lockfile lint pass. In the tests, only the 2src/core/image-data.test.tscases fail on my Mac, and they fail on unchangedmaintoo (/varvs/private/varpaths).tests/e2e/antipatterns.spec.tspasses (20 of 20).page-antipatterns-editor-wiring.test.ts: one section per group after 1 and 3 saves, the open detail showing the saved rule, and the detail closing when its rule is gone. All 4 fail onmain.