FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

docs(log-viewer): say what the row mark actually missed by lukecotter · Pull Request #976 · certinia/debug-log-analyzer · GitHub

docs(log-viewer): say what the row mark actually missed - #976

Merged
lcottercertinia merged 1 commit into
certinia:mainfrom
lukecotter:docs-row-mark-comment
Aug 28, 2026
Merged

docs(log-viewer): say what the row mark actually missed#976
lcottercertinia merged 1 commit into
certinia:mainfrom
lukecotter:docs-row-mark-comment

Conversation

Copy link
Copy Markdown
Collaborator

📝 PR Overview

#975 explained the row mark with a mechanism that does not exist: that the renderer de-initialises a row scrolled out of view and rebuilds it, dropping the class. A reader who trusts that comment would expect an ordinary scroll to lose a mark, and would look in the wrong place when the mark misbehaves.

What actually happens: Row.create() is guarded by this.created, Row.initialize() deletes cells but re-uses the element, RowManager.styleRow adds and removes parity classes rather than assigning className, and our renderer only detaches and re-attaches the element. A class on a row element survives scrolling. The gap the declarative mark closes is the first render: a row that has never been on screen has no element, so a sweep of what is rendered cannot reach it.

🛠️ Changes made

  • Correct the wantedByHost comment to name the real gap: a row below the viewport, or a tree child built after the mark was set.
  • Rename one test from "as a scroll back brings one" to "as scrolling to a new one does", and correct its helper comment, so the test says which case it guards.

🧩 Type of change (check all applicable)

  • 🐛 Bug fix - something not working as expected
  • ✨ New feature – adds new functionality
  • ♻️ Refactor - internal changes with no user impact
  • ⚡ Performance Improvement
  • 📝 Documentation - README or documentation site changes
  • 🔧 Chore - dev tooling, CI, config
  • 💥 Breaking change

📷 Screenshots / gifs / video [optional]

N/A.

🔗 Related Issues

Corrects comments added in #975.

✅ Tests added?

  • 👍 yes
  • 🙅 no, not needed
  • 🙋 no, I need help

Comments and one test name. The nine LocatedRowMarker tests still pass unchanged.

📚 Docs updated?

  • 🔖 README.md
  • 🔖 CHANGELOG.md
  • 📖 help site
  • 🧪 Marked any pre-release-only features
  • 🙅 not needed

Nothing user-visible changes.

Anything else we need to know? [optional]

No code changes: 6 insertions and 6 deletions, all inside comments and one it title.

The comment claimed the renderer rebuilds a row scrolled out of view and drops
its class. It does not: Row.create() is guarded, Row.initialize() re-uses the
element, and our renderer only detaches it. The gap is a row that has never
been rendered, so the sweep finds no element to mark.
lcottercertinia merged commit 5bc9dad into certinia:main Aug 28, 2026
7 checks passed
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL