(In reply to Alexandre Poirot [:ochameau] from comment #11) Alex is awesome! > I'm doing slightly more than that, I do merge requests-list-headers-wrapper and requests-list-headers. > The header-group should be an immediate child of <table> (in existing code and still in your patch there is a wrapper in between <table> and <thead>[<thead> is the header-group]) and the table-cell should be immediate children of the header-group (that is already the case) > This is important for clarity and performance and you will have to rebase against this particular setup. I don't understand this comment. 1) Proper table structure is as follows: ``` <table> <thead> <tr> <td>Status</td> <td>Method</td> </tr> </thead> <tbody> <tr> <td>200</td> <td>GET</td> </tr> </tbody> </table> ``` Lenka's patch is generating the following structure: ``` <div class="requests-list-table"> <table> display: table; table-layout: fixed; <div class="requests-list-headers-wrapper"> <thead> display: table-header-group;position: sticky; <div class="requests-list-headers"> <tr> display: table-row; position: relative; <div class="requests-list-column">Status</div> <td> display: table-cell; overflow: hidden; <div class="requests-list-column">Method</div> <td> display: table-cell; overflow: hidden; </div> </tr> </div> </thead> <div class="requests-list-contents"> <tbody> display: table-row-group; <div class="request-list-item"> <tr> display: table-row; // <<<<<< THIS CAUSES REGRESSION <div class="requests-list-column>200</div> <td> display: table-cell;overflow: hidden; <div class="requests-list-column">GET</div> <td> display: table-cell;overflow: hidden; </div> </tr> </div> </tbody> </div> ``` That is reasonable table structure and I don't see any extra elements. Alex's patch is doing: ``` <div class="requests-list-table"> <table> display: table <div class="requests-list-headers"> <thead> display: table-header-group; position:sticky <div class="requests-list-column"></div> <td> display: table-cell <div class="requests-list-column"></div> <td> display: table-cell </div> </thead> <div class="requests-list-row-group"> <tbody> display: table-row-group <div class="request-list-item"> <tr> display: table-row <div class="requests-list-column">200</div> <td> display: table-cell <div class="requests-list-column">GET</div> <td> display: table-cell </div> </tr> </div> </tbody> </div> </table> ``` Note that <thead> should have <tr> as the immediate child - this is missing. 2) Another thing, the status bar is not aligned at the bottom if there is e.g. just one request. This can be solved e.g. by having `height: 100%` on .requests-list-scroll element (if it isn't perf issue) 3) Auto scroll at the bottom is broken I'll be yet testing, but I don't see any issue atm. Thanks, Honza
Bug 1529018 Comment 13 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Alexandre Poirot [:ochameau] from comment #11) Alex is awesome! > I'm doing slightly more than that, I do merge requests-list-headers-wrapper and requests-list-headers. > The header-group should be an immediate child of <table> (in existing code and still in your patch there is a wrapper in between <table> and <thead>[<thead> is the header-group]) and the table-cell should be immediate children of the header-group (that is already the case) > This is important for clarity and performance and you will have to rebase against this particular setup. I don't understand this comment. 1) Proper table structure is as follows: ``` <table> <thead> <tr> <td>Status</td> <td>Method</td> </tr> </thead> <tbody> <tr> <td>200</td> <td>GET</td> </tr> </tbody> </table> ``` Lenka's patch is generating the following structure: ``` <div class="requests-list-table"> <table> display: table; table-layout: fixed; <div class="requests-list-headers-wrapper"> <thead> display: table-header-group;position: sticky; <div class="requests-list-headers"> <tr> display: table-row; position: relative; <div class="requests-list-column">Status</div> <td> display: table-cell; overflow: hidden; <div class="requests-list-column">Method</div> <td> display: table-cell; overflow: hidden; </div> </tr> </div> </thead> <div class="requests-list-contents"> <tbody> display: table-row-group; <div class="request-list-item"> <tr> display: table-row; // <<<<<< THIS CAUSES REGRESSION <div class="requests-list-column>200</div> <td> display: table-cell;overflow: hidden; <div class="requests-list-column">GET</div> <td> display: table-cell;overflow: hidden; </div> </tr> </div> </tbody> </div> ``` That is reasonable table structure and I don't see any extra elements. Alex's patch is doing: ``` <div class="requests-list-table"> <table> display: table <div class="requests-list-headers"> <thead> display: table-header-group; position:sticky <div class="requests-list-column"></div> <td> display: table-cell <div class="requests-list-column"></div> <td> display: table-cell </div> </thead> <div class="requests-list-row-group"> <tbody> display: table-row-group <div class="request-list-item"> <tr> display: table-row <div class="requests-list-column">200</div> <td> display: table-cell <div class="requests-list-column">GET</div> <td> display: table-cell </div> </tr> </div> </tbody> </div> </table> ``` Note that <thead> should have <tr> as the immediate child - this is missing. 2) Another thing, the status bar is not aligned at the bottom if there is just one request. This can be solved e.g. by having `height: 100%` on .requests-list-scroll element (if it isn't perf issue) 3) Auto scroll at the bottom is broken I'll be yet testing, but I don't see other any issue atm. Thanks, Honza