Svelte: add lines to directory groups#62981
Conversation
| </span> | ||
|
|
||
| <!-- We have to stop even propagation because the tree root listens for click events for | ||
| {#if expandable} |
There was a problem hiding this comment.
Slight change in behavior: instead of reserving space for the chevron icon so the file icon lines up with the folder icon, this lines up the file icon with the chevron of a sibling directory. This was necessary for the lines to make sense because we want the guideline to terminate at the file icon, not at an empty space in front of the file icon. This is visible in the screenshots.
| transform: translateX( | ||
| calc( | ||
| var(--tree-node-nested-level) * 1.25rem + var(--tree-node-left-padding) + var(--icon-inline-size) / | ||
| 2 - 1px | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Kinda a gnarly calculation, but it's using the same logic used to offset the labels within the list item.
taiyab
left a comment
There was a problem hiding this comment.
This looks great Camden. Awesome stuff! Really exciting update.
vovakulikov
left a comment
There was a problem hiding this comment.
Look good, I think there is only one small regression for the flat list items paddings
Compare
| Before | After |
|---|---|
I left a comment on how this might be fixed below.
| // since none of items is expandable, hence there aren't any offsets | ||
| :global([data-tree-view-flat-list='true']) & { | ||
| width: 0; | ||
| margin-left: 0.5rem; |
There was a problem hiding this comment.
This logic previously was giving additional left spacing for the flat list case
See my comment with screenshots above. I think we should add a special value
for the flat list selector
[role="treeitem"] {
/* ... styles .... */
:global([data-tree-view-flat-list='true']) & {
--tree-node-left-padding: 2.75rem;
}
}There was a problem hiding this comment.
Ah, I see now! I was struggling to figure out how data-tree-view-flat-list was actulaly triggered. Will fixup 👍
There was a problem hiding this comment.
Okay, fixed it. Now that we're not specially indenting files (compared to dirs), the calculation actually got quite a bit simpler. I think there are also some followup simplifications we could make to remove the absolute positioning and padding calculations.
There was a problem hiding this comment.
Looks like we don't use data-tree-view-flat-list anywhere else, so we can remove the logic that sets it in TreeView.
There was a problem hiding this comment.
Yes! Planning a followup PR with simplifications. I started doing it in this PR, but it got pretty noisy
| // since none of items is expandable, hence there aren't any offsets | ||
| :global([data-tree-view-flat-list='true']) & { | ||
| width: 0; | ||
| margin-left: 0.5rem; |
There was a problem hiding this comment.
Looks like we don't use data-tree-view-flat-list anywhere else, so we can remove the logic that sets it in TreeView.
After the changes to the file tree layout in #62981, we can clean up the styles and DOM a bit. These changes are partially in prep for experimenting with alternative "current path" representations. Comments inline.
This adds guidelines for open directories in the file tree. I recommend reviewing with whitespace changes hidden.
Fixes SRCH-443
Dark:
Light:
Test plan
Manual testing. This is one where a screenshot test would be really nice 🙂