Use a flex layout for the tree-view-resized div

[abe33]

This change does not impact how the tree-view render, but will allow
other packages to insert new content in the tree-view-resizer div
without having to worry about the layout being broken or the list not
scrolling to the last item (as the list can be pushed by the additional
content).

For instance autohide-tree-view sets the height using something like
calc(100% - 29px) which maybe fine if it’s the only package. But it
starts to get ugly when different packages insert content in the
tree-view with different methods (see
abe33/atom-tree-view-breadcrumb#19 for instance).

👎 Broken Packages

• tree-view-open-files But it seems already broken before

👋 "Ok, but could be improved" Packages

• opened-files: Still works, but the inline (auto) height could be replaced with flex so that other packages can be added. Auto height paulpflug/opened-files#16
• tree-view-breadcrumb: Could use official classes or update the order

👍 Unaffected Packages

• autohide-tree-view (except the pin could clash with other stuff at the top)

Note: The above is only true if each package gets added alone. There are more (sometimes unrelated) issues when enabling multiple packages together.

[izuzak]

cc @simurai for 👀

[simurai]

👍 on changing to flexbox.

But it starts to get ugly when different packages insert content in the tree-view with different methods

Right. I tried out open-files and it changes the height of the tree-view, but doesn't know of other packages.

I guess we can remove .tree-view-scroller { height: 100%; } and use flex: 1 instead. Then other packages can either leave their height to auto, or also use flex: 1 to share the space 50/50 or so.

I'll try that out.

[simurai]

@abe33 Should we merge #578 into this PR since this is dependant on it?

[abe33]

Should we merge #578 into this PR since this is dependant on it?

It would be better as this change won't be visible as long as we force a display block on the tree view element.
I wanted to make it two separate PR as the first is mostly a fix to avoid pollution, this one being more a change for the future.

Also tried tree-view-open-files but I get an error

Yes I got one too, there was a change in the activatePackage method that broke tree-view-breadcrumb and tree-view-open-files. Looks like @postcasio isn't around lately, I'll create an issue for him about that and about the flex layout.

For autohide-tree-view I think the best solution for the pin button would be to also use the flex layout.

[abe33]

@simurai What do you think about specifying a value for the tree-view-resizer's order property so that packages can decide whether to add their content above or below the tree-view without having to touch to the tree-view-resizer styles?

Actually that's what I do for in the breadcrumb package because I don't want to rely on the node order (I use an order of 2 for the tree-view-resizer, 1 for the breadcrumb so that element without an order are displayed above both the breadcrumb and the tree-view).

[simurai]

Ya, that's a good idea. Should it be done with classes? To make it feel more "API like":

.tree-view--top    { order: 0; }
.tree-view--center { order: 1; }
.tree-view--bottom { order: 2; }

The tree-view would be in the center: <div class="tree-view-scroller tree-view--center">. I guess if two packages use the same, then the DOM order decides again. Kinda like z-index.

or attributes:

.tree-view-resizer > [data-position="top"]    { order: 0; }
.tree-view-resizer > [data-position="center"] { order: 1; }
.tree-view-resizer > [data-position="bottom"] { order: 2; }

Or use a more generic start, center, end, just so it could be used in other places, not just the tree-view, also horizontally.

.tree-view-resizer > [data-position="start"]  { order: 0; }
.tree-view-resizer > [data-position="center"] { order: 1; }
.tree-view-resizer > [data-position="end"]    { order: 2; }

Or or or.. 😆 Does it need even steps in between:

.tree-view-resizer > [data-position="start"]         { order: 0; }
.tree-view-resizer > [data-position="before-center"] { order: 1; }
.tree-view-resizer > [data-position="center"]        { order: 2; }
.tree-view-resizer > [data-position="after-center"]  { order: 3; }
.tree-view-resizer > [data-position="end"]           { order: 4; }

Like when you don't really care to be the top item, but you just wanna make sure you're above the tree-view, you could add data-position="before-center".

[abe33]

I guess if two packages use the same, then the DOM order decides again.

Yes, that's how it works.

Should it be done with classes or attributes?

I don't really have any preferences between classes and attributes. Maybe attributes are a bit more verbose than classes.

Or use a more generic start, center, end, just so it could be used in other places, not just the tree-view, also horizontally.

Yeah, sounds more robust, and probably in that case, if we want to use classes then we can use a more generic name like .flex-order--start, .flex-order--center, etc.

Does it need even steps in between

I think so. As 0 is the default order, you can force to be positionned before an element with order=0 using a negative order but you can't target the space just before the tree-view with only 3 steps.

[simurai]

Or just order--xxx? More general not really tied to flexbox.

I tried to think of a good name for the "in-betweens", but kinda failed. Maybe this:

.order--start        { order: 0; }
.order--start-center { order: 1; }
.order--center       { order: 2; }
.order--center-end   { order: 3; }
.order--end          { order: 4; }

[simurai]

Ok, I pushed this:

.tree-view-resizer {
  & > .order--start  { order: -10; }
  & > .order--center { order:   0; }
  & > .order--end    { order:  10; }
}

1. The .order--xxx classes are initially scoped to be a child of .tree-view-resizer. If ok, it might can be used globally.
2. .order--center is 0, the same as the default. I think this is handy because it doesn't change how currently packages are ordered. So adding the classes is only opt-in for those who want and doesn't break existing behaviour.
3. Using a range from -10 to 10 still leaves packages room to target the approximate order. Like if my package should be somewhere towards the end, I could use 6 or so.

[abe33]

Looks fine to me 👍

Maybe we could list the packages that append content to the tree-view and notify them of the changes ?

[simurai]

Maybe we could list the packages that append content to the tree-view and notify them of the changes ?

Added a few to the top. Will try to find some more. So far I think it's ready to be merged since it doesn't totally break any.

[simurai]

@abe33 These changes are now published. Not sure when it's actually gonna land on stable.

[abe33]

@simurai Ok, thank you for the help. I'll keep an eye on that and I will prepare tree-view-breadcrumb for the change.

[olmokramer]

I noticed an issue with the current approch: tree view context menu entries use the .tree-view selector, but as it is right now the .tree-view doesn't take all the available vertical space. So when I right-click on the area below the .tree-view the context menu is empty.

[thomasjo]

@olmokramer Noticing the same. Would you mind opening a separate bug report for this? If not, I can do it later.

[olmokramer]

@olmokramer Noticing the same. Would you mind opening a separate bug report for this? If not, I can do it later.

Sure: #595 :)

[rugk]

autohide-tree-view does not really seem to be that unaffected... possibly...
olmokramer/atom-autohide-tree-view#62