Conversation
telamonian
left a comment
telamonian
left a comment
There was a problem hiding this comment.
I like it so far! A nice cleanup of the existing example that should be easier for someone new to follow.
Challenge: is there an equally nice way to implement the tree sort? Is there a simple sort implementation that keeps the pattern of modifying DATA in place?
In any case the sort stuff can be put off until the next PR, but I am curious
| function styleListener() { | ||
| for (const td of window.regularTable.querySelectorAll("tbody th")) { | ||
| const meta = window.regularTable.getMeta(td); | ||
| td.classList.toggle("fb-directory", meta.value && DATA[meta.y].row[1] === "directory"); |
There was a problem hiding this comment.
| td.classList.toggle("fb-directory", meta.value && DATA[meta.y].row[1] === "directory"); | |
| td.classList.add(meta.value && DATA[meta.y].row[1] === "directory" ? "fb-directory" : "fb-file"); |
The browser looks much more homey when there's both dir and file icons, so we should handle classes for both here. More verbosely, you could instead do
td.classList.toggle("fb-directory", meta.value && DATA[meta.y].row[1] === "directory");
td.classList.toggle("fb-file", meta.value && DATA[meta.y].row[1] === "file");
add is safe to use here, since the table cell classes get blanked on every redraw, right? Actually, that's a good question: does any table cell state carry over from invocation to invocation of the style listeners?
There was a problem hiding this comment.
It hasn't been decided what regular-table's responsibility vis-a-vis resetting <td> state is yet. Ideally it would be none, but className and style are used currently by the internal auto-size and override code.
|
|
||
| ```javascript | ||
| function styleListener() { | ||
| for (const td of window.regularTable.querySelectorAll("tbody th")) { |
There was a problem hiding this comment.
Love the use of metadata in this loop, much cleaner/easier to follow than before
| tbody th:not(.fb-directory):not(:empty) { | ||
| padding-left: 20px; | ||
| } |
There was a problem hiding this comment.
| tbody th:not(.fb-directory):not(:empty) { | |
| padding-left: 20px; | |
| } | |
| tbody th:fb-file { | |
| font-family: "Material Icons"; | |
| content: "text_snippet "; | |
| } |
The rest of the implementation for a file icon. Why did you put a white space at the end of the other icon ligatures?
texodus
force-pushed
the
file-browser-rewrite
branch
13 times, most recently
from
1d589bf to
8281171
Compare
texodus
force-pushed
the
file-browser-rewrite
branch
from
8281171 to
a38417c
Compare
| regularTable.addStyleListener(styleListener); | ||
| regularTable.addEventListener("mousedown", mousedownListener); | ||
| regularTable.addEventListener("scroll", () => { | ||
| regularTable.resetAutoSize(); |
There was a problem hiding this comment.
typo, missing underscore. Should be
regularTable._resetAutoSize();
right? Super helpful that I spotted this right after you merged
2 participants
No description provided.