Skip to content

Hide a filling table's colgroup on narrow pages instead of !important - #39

Open
HarryCordewener wants to merge 1 commit into
mainfrom
claude/adopt-layout-functions-hc9kwm
Open

HarryCordewener wants to merge 1 commit into
mainfrom
claude/adopt-layout-functions-hc9kwm

Conversation

@HarryCordewener

@HarryCordewener HarryCordewener commented Oct 7, 2026 •

Copy link
Copy Markdown
Member

Before: on a page under 48em, LayoutCss gave up a filling table's column shares with .ms-table.ms-fill > colgroup > col { width: auto !important; }. SharpMUSH's portal copies LayoutCss.Fixed into shell.css and refuses !important in its global stylesheets, so its two tests contradict each other and SharpMUSH #1648 can't go green on 2.11.0.

After: the rule is .ms-table.ms-fill > colgroup { display: none; }. In Chromium at 600px, an 80%/20% colgroup lays out at 348/235 px. With the colgroup hidden it lays out at 69/514, the same as a table with no colgroup.

How: one CSS rule and its test. The changelog gets the 2.11.0 heading the grow entry shipped under, with this fix under Unreleased. It needs a 2.11.1 release for SharpMUSH to pick up.

🤖 Generated with Claude Code

https://claude.ai/code/session_018eAjtsQTFqWAeWQMc7ebNo


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • On narrow screens, filling tables now hide their column group when the columns can’t fit, rather than forcing column widths.

…g its widths with !important

The 48em rule gave up the columns' shares with `col { width: auto !important; }`. Hiding the
colgroup drops the inline widths the same way (checked in Chromium: a 80%/20% colgroup at 600px
lays out 348/235 with it, 69/514 hidden, the same as no colgroup), and a host whose stylesheets
refuse !important, such as SharpMUSH's portal, can carry the rule as written. The changelog also
gets the 2.11.0 heading the grow entry shipped under.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018eAjtsQTFqWAeWQMc7ebNo
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Essentials
  • Run ID: cadc3434-5126-4712-8ddc-8fc7d63c18e7
📥 Commits

Reviewing files that changed from the base of the PR and between dd51634 and 7d22b04.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • MarkupString.Html/LayoutCss.cs
  • MarkupString.Tests/Layout/WidgetLayoutTests.cs

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Your free on-demand review promotion remains active until October 9, 2026 at 6:00 PM UTC.


Walkthrough

At viewport widths up to 48em, filled tables now hide their <colgroup> instead of setting its columns to automatic widths. The test expectation and Unreleased changelog entry reflect this change.

Changes

Filled table layout

Layer / File(s) Summary
Hide filled table column groups
MarkupString.Html/LayoutCss.cs, MarkupString.Tests/Layout/WidgetLayoutTests.cs, CHANGELOG.md
The narrow-width CSS rule hides the <colgroup> for filled tables. The test now expects it to be hidden, and the changelog records the behavior. The changelog also adds the 2.11.0 release heading dated 2026-10-07.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: claude

Merge Risk: ⚪ Minimal · up to 7d22b

The narrow-page table change is reflected in its test and changelog. No actionable merge risk is established by the supplied changes; proceed with normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: hiding a filling table’s colgroup on narrow pages instead of using !important.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

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