-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
File Browser II #57
File Browser II #57
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
examples/file_browser.md
Outdated
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
examples/file_browser.md
Outdated
|
||
```javascript | ||
function styleListener() { | ||
for (const td of window.regularTable.querySelectorAll("tbody th")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the use of metadata in this loop, much cleaner/easier to follow than before
examples/file_browser.md
Outdated
tbody th:not(.fb-directory):not(:empty) { | ||
padding-left: 20px; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
1d589bf
to
8281171
Compare
8281171
to
a38417c
Compare
regularTable.addStyleListener(styleListener); | ||
regularTable.addEventListener("mousedown", mousedownListener); | ||
regularTable.addEventListener("scroll", () => { | ||
regularTable.resetAutoSize(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo, missing underscore. Should be
regularTable._resetAutoSize();
right? Super helpful that I spotted this right after you merged
No description provided.