fix: table styling#7712
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates common SCSS styling to (1) remove reliance on the client-js class for collapsible behaviors and (2) adjust table border rendering by adding border-collapse: collapse.
Changes:
- Added a global
border-collapse: collapserule intended to prevent “double/thick” table borders. - Removed
html.client-jsgating from Prizepooltable collapsed-row hiding rules. - Removed
html.client-jsgating from NavFrame and generic.collapsiblecollapsed-content hiding rules.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| stylesheets/commons/Tables.scss | Adds border-collapse: collapse rule for tables/wikitables. |
| stylesheets/commons/Prizepooltable.scss | Changes collapsed-row hiding selector from html.client-js to html. |
| stylesheets/commons/NavFrame.scss | Changes NavFrame/collapsible collapsed-content hiding selectors from html.client-js to html. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| @for $i from 0 through 32 { | ||
| html.client-js .prizepooltable.collapsed[ data-cutafter="#{$i}" ] tr:nth-child( n + #{ $i + 3 } ) { | ||
| html .prizepooltable.collapsed[ data-cutafter="#{$i}" ] tr:nth-child( n + #{ $i + 3 } ) { |
| } | ||
|
|
||
| html.client-js .NavFrame.collapsed .NavContent { | ||
| html .NavFrame.collapsed .NavContent { |
| html .collapsible.collapsed > thead > tr:nth-child( n+2 ), | ||
| html .collapsible.collapsed > thead + tbody, | ||
| html .collapsible.collapsed > tbody > tr:nth-child( n+2 ), | ||
| html .collapsible.collapsed > tfoot, | ||
| html .collapsible.collapsed > tr:nth-child( n+2 ) { |
| table, | ||
| .wikitable { | ||
| border-collapse: collapse; | ||
| } |
There was a problem hiding this comment.
this was following the current production css rules that has table border-collapse as collapse
|
as copilot already mentioned |
I ask around and the PO and few devs said that supporting disabled JS browser wasn't a priority anymore, since most of the site functionality dependent on JS now, so I think we could safely ignore the concern right now. @Rathoz @FO-nTTaX |
yeah since so much other stuff these days depends on js I don't think there's much of a point at attempting this, I would just remove the nojs stuff eventually |
Summary
On Lighthouse the
.client-jsclass is missing (i am not sure about the significance), so this PR removes it and make it so that the styles is deteced by lighthouse. Also it adds aborder-collapsestyling so that the table doesnt have a thick border.How did you test this change?
I removed
.client-json the production wiki by overriding the style and it works.