Skip to content

Commit

Permalink
Update header.php
Browse files Browse the repository at this point in the history 
Remove deprecated/obsolete styling on main table/container (table id="layout")
  • Loading branch information
@extremecarver
extremecarver authored Jul 5, 2023
1 parent 8d6cf15 commit e83c9bf
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions header.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
<?php } ?>
<div id="wrapper">
<div id="container">
<table id="layout" border="0" cellspacing="0" cellpadding="0">
<table id="layout">
<colgroup>
<?php if ( $left_col == "on" ) { ?><col class="colone" /><?php } ?>
<?php if ( $left_col2 == "on" ) { ?><col class="colone-inner" /><?php } ?>
Expand Down Expand Up @@ -154,4 +154,4 @@


<!-- Main Column -->
<td id="middle">
<td id="middle">

6 comments on commit e83c9bf

@lx1
Copy link

@lx1 lx1 commented on e83c9bf Aug 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change introduces an unpleasant 2px border around the whole page, at least for me and at least on Chrome and Firefox. I'm no CSS expert but now it seems that the UA built-in stylesheet border-spacing: 2px now takes place, where in the past it was zeroed-out by the old syles that where removed by this commit. Maybe border-spacing: 0px should be added to some element like table or table#layout to preserve the previous behaviour? Or are there better solutions?

@rivella50
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@extremecarver What do you think?

@extremecarver
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes by default you will get:
display: table;
border-collapse: separate;
box-sizing: border-box;
text-indent: initial;
border-spacing: 2px;
border-color: gray;

with chrome. However if you need that 0px you can just get it back by adding a CSS insert with 0... However otherwise it's impossible to add any custom spacing/padding as this will overwrite it. It's not good in general to have hardcoded CSS in a theme that is supposed to be fully adaptable IMHO. The problem is that the table layout will be inherited downwards by other elements and also those you can then not overwrite further complicating it.

@rivella50
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but if everyone now encounters this 2px border spacing (and doesn't want it) then i wonder if we shouldn't go back to the previous version since the 0px version is the one that most devs/users expect by default.

@lx1
Copy link

@lx1 lx1 commented on e83c9bf Aug 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@extremecarver , I agree that it's preferable to lave static CSS out from a customizable theme but, since this changes something that had been like this for a long time, I would at least put a note in the changelog telling strictly what to do to have the old behaviour. Something similar to the statement about the removal or display:block from div.widget ul li (another change in the behaviour I stubled upon, but it was documented so it was easy to fix).

@extremecarver
Copy link
Collaborator Author

@extremecarver extremecarver commented on e83c9bf Aug 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the border spacing with my layout so I'm not sure how to verify. But the following CSS Insert in Atahualpa options should overwrite the browser default:
table#layout {border-spacing: 2px; border:0px;}
Actually maybe leave out border spacing, I'm not sure what was the default here.

Please sign in to comment.