Fix empty vertical scrollbar in PR comment diff table - #1078
Open
TovaBecker wants to merge 2 commits into
Open
TovaBecker wants to merge 2 commits into
TovaBecker wants to merge 2 commits into
Conversation
Each result row ended with a stray `<tr>` instead of `</tr>`, and the table was never closed with `</tbody></table>`. Browsers silently "fixed" this malformed markup by inserting a phantom empty row, which threw off GitHub's accessible-table wrapper and produced an empty vertical scrollbar with nothing to scroll. Fixes github-community-projects#1077 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is small, localized, and directly addresses the malformed markup causing the rendering issue.
Pull request overview
This PR fixes malformed HTML emitted in the “Safe-Settings config changes detected” PR comment diff table so GitHub’s rendering no longer shows an empty vertical scrollbar.
Changes:
- Fixes table row markup by closing each row with
</tr>instead of a stray<tr>. - Properly closes the diff table with
</tbody></table>after rendering all rows.
File summaries
| File | Description |
|---|---|
| lib/settings.js | Corrects generated PR-comment diff table HTML by closing <tr> rows and the overall table. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot flagged that the scrollbar fix had no test guarding against the malformed markup reappearing. Assert the generated comment body never contains the stray </td><tr> pattern and always closes the table with </tbody></table>. Verified this test fails against the pre-fix code and passes against the fix. Addresses review comment from github-community-projects#1078. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to HTML tag correctness, includes a focused unit test, and does not alter the surrounding comment-generation behavior beyond fixing malformed markup.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
This branch has not been deployed
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.
Summary
<tr>instead of</tr>, and the table was never closed with</tbody></table>. Browsers silently "fixed" this malformed markup by inserting a phantom empty row, which threw off GitHub's<markdown-accessiblity-table>wrapper component's scroll-affordance measurement.</tr>, and close the table with</tbody></table>.Test plan
npm run test:unitpasses (37/37 intest/unit/lib/settings.test.js), no new lint violations.Accept: application/vnd.github.html+json) before and after the fix — confirmed the phantom<tr></tr>is gone and the scrollbar no longer appears.