Skip to content

Adds file_browser.html example (update for row_headers feature)#25

Merged
texodus merged 2 commits intomasterfrom
file-browser-row-headers
May 30, 2020
Merged

Adds file_browser.html example (update for row_headers feature)#25
texodus merged 2 commits intomasterfrom
file-browser-row-headers

Conversation

@texodus
Copy link
Copy Markdown
Member

@texodus texodus commented May 29, 2020

Updates #19 from @telamonian to rely on the new row_headers feature introduced in #24 . The add-filebrowser-example branch was first rebased:

  • Commit history was squashed to 1 commit
  • Changes to tree_row_header.js and 'events.js` were removed.
  • Changes to perspective.html were removed (which relied on the above).
  • README.md changes reverted (no need to add your example to the gallery - I'll do this automatically via script when I dist).

Next, the example was refactored. Unfortunately some of my comments from the original PR were quickly outdated by rapid progress on this API.

  • Removes __config, __collapse and __expand pass-through.
  • Uses row_headers instead of row_indices, et al columns.
  • Adds a custom tree_header renderer ala perspective.html, modified to handle the lack of an empty 'root' node.
  • Adds a click handler to dispatch to collapse and expand()
  • Refactors code into 2 script sections - "fake filesystem generator" and "data model for regular-table" sections.
  • General cleanup and lint-powered reformat.

Screen Shot 2020-05-29 at 6 43 41 PM

@texodus texodus requested a review from telamonian May 29, 2020 22:44
@texodus texodus changed the title Adds a File browser example (update for row_headers feature) Adds file_browser.html example (update for row_headers feature) May 29, 2020
Copy link
Copy Markdown
Contributor

@telamonian telamonian left a comment

Choose a reason for hiding this comment

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

LGTM!

The only other thing I'd like to get in as part of this PR is a way to set a text title on the names column (ie the row_headers column). You can sort of do it via row_pivots, but a) that's obviously not what it's for and b) in that case the column name still isn't visible. It's not a blocker, though

Comment thread src/less/material.less

td th span.pd-group-leaf, th span.pd-group-leaf {
margin-left: 12px;
margin-left: 16px;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a fix for the issue where the <span class="pd-row-header-icon"> elements were throwing off the alignment of the dir/file names, right? Nice work

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes - Perspective has no way to generate datasets with "folders" at the same level as "leaves", so this number was off a few pixels.

@texodus
Copy link
Copy Markdown
Member Author

texodus commented May 30, 2020

Thanks for the review! I will think about concise ways to name the "corner" as well, PRs welcome.

@texodus texodus merged commit ef619cc into master May 30, 2020
@texodus texodus deleted the file-browser-row-headers branch May 30, 2020 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants