abe33 / atom-tree-view-breadcrumb

Display a breadcrumb on tree view scroll
MIT License
10 stars 3 forks source link

Completely breaks tabs bar #19

Open ProLoser opened 8 years ago

ProLoser commented 8 years ago
screen shot 2015-08-27 at 4 04 36 pm
ProLoser commented 8 years ago
apm list
Built-in Atom packages (89)
├── about@1.0.1
├── archive-view@0.58.0
├── atom-dark-syntax@0.27.0
├── atom-dark-ui@0.50.0
├── atom-light-syntax@0.28.0
├── atom-light-ui@0.43.0
├── autocomplete-atom-api@0.9.2
├── autocomplete-css@0.10.1
├── autocomplete-html@0.7.2
├── autocomplete-plus@2.19.0
├── autocomplete-snippets@1.7.1
├── autoflow@0.25.0
├── autosave@0.22.0
├── background-tips@0.26.0
├── base16-tomorrow-dark-theme@0.26.0
├── base16-tomorrow-light-theme@0.9.0
├── bookmarks@0.36.0
├── bracket-matcher@0.76.0
├── command-palette@0.36.0
├── deprecation-cop@0.54.0
├── dev-live-reload@0.46.0
├── encoding-selector@0.21.0
├── exception-reporting@0.36.0
├── find-and-replace@0.180.0
├── fuzzy-finder@0.87.0
├── git-diff@0.55.0
├── go-to-line@0.30.0
├── grammar-selector@0.47.0
├── image-view@0.54.0
├── incompatible-packages@0.24.1
├── keybinding-resolver@0.33.0
├── language-c@0.47.0
├── language-clojure@0.16.0
├── language-coffee-script@0.41.0
├── language-csharp@0.7.0
├── language-css@0.33.0
├── language-gfm@0.80.0
├── language-git@0.10.0
├── language-go@0.37.0
├── language-html@0.40.1
├── language-hyperlink@0.14.0
├── language-java@0.16.0
├── language-javascript@0.87.1
├── language-json@0.16.0
├── language-less@0.28.2
├── language-make@0.16.0
├── language-mustache@0.12.0
├── language-objective-c@0.15.0
├── language-perl@0.28.0
├── language-php@0.29.0
├── language-property-list@0.8.0
├── language-python@0.38.0
├── language-ruby@0.57.0
├── language-ruby-on-rails@0.22.0
├── language-sass@0.40.1
├── language-shellscript@0.15.0
├── language-source@0.9.0
├── language-sql@0.17.0
├── language-text@0.7.0
├── language-todo@0.26.0
├── language-toml@0.16.0
├── language-xml@0.32.0
├── language-yaml@0.24.0
├── link@0.30.0
├── markdown-preview@0.150.0
├── metrics@0.51.0
├── notifications@0.57.0
├── one-dark-syntax@1.1.0
├── one-dark-ui@1.0.3
├── one-light-syntax@1.1.0
├── one-light-ui@1.0.3
├── open-on-github@0.38.0
├── package-generator@0.40.0
├── release-notes@0.53.0
├── settings-view@0.213.0
├── snippets@0.95.0
├── solarized-dark-syntax@0.38.1
├── solarized-light-syntax@0.22.1
├── spell-check@0.59.0
├── status-bar@0.75.1
├── styleguide@0.44.0
├── symbols-view@0.100.0
├── tabs@0.82.0
├── timecop@0.31.0
├── tree-view@0.183.0
├── update-package-dependencies@0.10.0
├── welcome@0.30.0
├── whitespace@0.30.0
└── wrap-guide@0.35.0

/Users/Dean/.atom/packages (52)
├── .bin
├── Sublime-Style-Column-Selection@1.3.0
├── advanced-new-file@0.4.3
├── atom-beautify@0.28.11
├── atom-prettify@0.1.3
├── atom-typescript@5.5.1
├── atomatigit@1.5.4
├── auto-reveal-in-sidebar@0.5.0
├── auto-update-packages@1.0.0
├── autoclose-html@0.18.0
├── autocomplete-paths@1.0.2
├── autocomplete-ruby@0.1.0
├── autohide-tree-view@0.24.3
├── blame@0.5.0
├── color-picker@2.0.11
├── docblockr@0.7.3
├── emmet@2.3.12
├── file-icons@1.6.9
├── floobits@0.11.2
├── git-control@0.3.0
├── git-diff-details@0.20.0
├── git-difftool@0.2.6
├── git-go@1.0.1
├── git-history@3.1.0
├── git-log@0.4.1
├── html-entities@0.4.0
├── jscs-fixer@1.0.0
├── language-diff@0.3.1
├── language-haml@0.21.0
├── line-diff-details@1.1.2
├── linter@1.4.3
├── linter-jshint
├── linter-rubocop@0.3.3
├── linter-scss-lint@2.0.0
├── livereload@0.3.3
├── merge-conflicts@1.3.5
├── minimap@4.13.1
├── minimap-autohide@0.10.1
├── minimap-find-and-replace@4.3.0
├── minimap-git-diff@4.1.8
├── open-git-modified-files@0.2.1
├── pane-layout-plus@0.7.3
├── pigments@0.11.0
├── react@0.12.6
├── rubocop-auto-correct@1.0.0
├── sublime-word-navigation@0.1.2
├── tabs-closer@0.13.0
├── toggle-quotes@0.11.3
├── tree-view-breadcrumb@0.6.2
├── tree-view-git-branch@0.0.2
├── tualo-git-context@0.6.14
└── zentabs@0.8.6
abe33 commented 8 years ago

Are you sure it's the one package that breaks your tabs? I tried to replicate your setup (tree-view on right and same breadcrumb settings) and I couldn't reproduce it.

rugk commented 8 years ago

I'm experincing this issue too. But it's hard to reproduce and only happens sometimes. (it seems to depend on the repo/directory which I open in Atom) The first time I show the treeview it hides my tabs. My tree view is at the left and I'm (also) using autohide-tree-view to hide it. So I trigger it by moving the mouse to one side.

If I disable autohide-tree-view or this package it works. After I enable autohide-tree-view again - without restarting Atom - and hover my mouse at the left so the treeview is shown it hides the tabs. However if I disable this package I can use autohide-tree-view without problems. So there seems to be a connection between these two packages which is causing this.

Windows 7 Atom 1.0.11 authide-tree-view v 0.24.3

CC: @olmokramer (author of autohide-tree-view) Atom 1.0.11

abe33 commented 8 years ago

Hmm, interesting, @rugk thanks for the explanation, I haven't autohide-tree-view so that may explain why I don't reproduce it. I'll take a look at that.

rugk commented 8 years ago

Note that it also affects editor-stats when this issue occurs. It renders the panel ineffectively and where the time is normally shown there is only a grey rectangle afterwards.

rugk commented 8 years ago

Did you tested it?

abe33 commented 8 years ago

Not yet, sorry, I was out this week-end for a geekcamp with fellow developpers from Bordeaux. I'll check that soon though.

abe33 commented 8 years ago

Ok, so I could take some time to test the various packages listed in this ticket, and sadly I don't seem to reproduce it at least once.

If someone ca take sometime to look at the markup when the tabs disappear so that can grasp the situation. Is it that the tabs are hidden (style conflict) or that the tab view is removed from the DOM?

ProLoser commented 8 years ago

I believe the reliable way to reproduce this is simply folding closed the top-level folder. This appears to reliably break atom's gui

abe33 commented 8 years ago

Ok, I finally found a way to reproduce that. It looks like the main reason is a change in the tree-view package that forces a block display on the tree view to refresh the scrollbars styles. IIRC it was inheriting a flex layout before that both tree-view-breadcrumb and other package like tree-view-open-files were relying upon. With a block display and the fact that autohide-tree-view is forcing a computed height on the tree view create a mess in the styles that push everything up and hides the tab view (it also affect the tree view scroll as the list is no longer shrinked by the other flex elements).

As is I can't force the tree view to use a flex display as it would break the fix for scrollbar refresh. And I think autohide-tree-view should ideally use a flex display too to insert the pin button (each package cannot assume it'll be the only one to append stuff in the tree view so we need to get smarter about that).

I'll try to tweak the tree-view to see if it updates the scrollbars without relying on a display block style. And if it goes well I'll submit a PR to tree-view and autohide-tree-view to fix that in one go.

ProLoser commented 8 years ago

:+1: your effort is very much appreciated

olmokramer commented 8 years ago

@abe33 Nice one! I never thought of that! Please, let me know if you have any suggestions for changes in autohide-tree-view.

abe33 commented 8 years ago

@olmokramer I submitted a PR to tree-view to enforce the use of a flex layout. That way you'll no longer need to change the size of the tree-view, just use a flex display for the pin button (or better, on its container, so that you can position the button properly inside it) and everything should be fine.

olmokramer commented 8 years ago

@abe33 the reason I set a height on the tree view is so it won't cover the status bar when it expands... Also, I don't understand why I should set the display of the pin button to flex. I just want it absolutely positioned in the top-right corner.

abe33 commented 8 years ago

@olmokramer Yes I understand why you did it, in fact I did the same in an early version of the breadcrumb. By using a flex layout we can add content to the tree-view-resizer and at the same we're sure that the tree-view-scroller will just take the remaining space by itself (and won't overlap the status bar).

For the pin button, the use of a flex layout will ensure that you won't create a conflict with another package: what if a package adds a toolbar on the top of the tree-view? Using an absolute positionning you may end up covering a button from this toolbar. With a flex-layout you just have to care about the order you give to your element to position it in the tree-view-resizer.

olmokramer commented 8 years ago

tree-view-status-bar

tree-view-status-bar2

The first image is with a height set on the .tree-view-resizer, the second without. I don't see how a flex layout will help with that...

About the pin button, that would mean that the pin button will push down the other elements in the tree view, no? I really like how it's horizontally aligned with the project name.

olmokramer commented 8 years ago

PS: That said, I think I'm going to remove the height from the .tree-view-resizer anyway, because it's less useful than I'd expected...

abe33 commented 8 years ago

Oh! Sorry I wasn't thinking about this use case, then you're probably right to specify the height of the tree-view-resizer div. However it doesn't change the fact that a flex layout will solve most of our problems when many packages append content to the tree view. Anyway it's an interesting problem and I'm pretty sure @simurai may have some insight about this kind of things.

simurai commented 8 years ago

There is an issue about making the status-bar go full width: https://github.com/atom/status-bar/issues/91. Depends on https://github.com/atom/atom/issues/8939. Not sure if/when that's gonna happen, but that might help with not having to calculate the height.

Re. pin button.. currently if both packages are installed, then the pin button gets overlapped by atom-tree-view-breadcrumb and can't be clicked.

screen shot 2015-09-29 at 5 54 38 pm

Maybe there could be a guide:

If there is a potential of overlapping, child elements of packages should stay within the boundaries of the package.

In this case the pin button would be anchored to the tree-view boundaries:

screen shot 2015-09-29 at 7 44 49 pm

It's a bit more complex because atom-tree-view-breadcrumb actually injects itself into .tree-view-resizer. So there are 2 packages inside another package that don't really know about each other. So yeah, it's tricky.

olmokramer commented 8 years ago

Aha, I see. That's doesn't work very well :( I've made changes to autohide-tree-view in https://github.com/olmokramer/atom-autohide-tree-view/pull/56 to prepare for tree-view v0.190.0. In the end I just went with the flex ordering and set the pin button's order: -10 so it will be at the top of the tree view.

rugk commented 8 years ago

BTW this tab issue also appears when you change the setting "Show on the right" in the options of Atom's package tree-view.