eoger / tabcenter-redux

Vertical Tabs extension for Firefox
Mozilla Public License 2.0
381 stars 67 forks source link

Support Tab Hiding #296

Closed kah0922 closed 6 years ago

kah0922 commented 6 years ago

Initial support for tab hiding has landed as of the 1-19 build of Nightly. Currently, only Conex has enabled support for this.

Currently, Tab Center Redux will show all tabs regardless of whether they are hidden (via Conex or when other Tab addons add support) or not.

eoger commented 6 years ago

Yes, this is definitely needed.
Things to do on the top of my head:

thomcc commented 6 years ago

Is issue this just to show/hide these tabs in the sidebar (e.g. to behave well in the face of another extension that uses this api), or is it to add the ability to hide tabs in some manner

ajvsol commented 6 years ago

I think the former as the extension author has said they're not planning to add Tab Groups right now, and this is likely to be the main use of the API.

Smile4ever commented 6 years ago

A context menu item to hide the a tab from the sidebar using the WebExtensions APIs would probably be needed as well.

gee-forr commented 6 years ago

I was wondering... would a good first pass of this functionality be to just merely only show non-hidden tabs instead of implemented full show/hide functionality?

I suspect that anyone currently hiding tabs are doing so via Conex, and will continue to do so via Conex & co, and really just want TabCenter redux for the sidebar.

eoger commented 6 years ago

I just pushed a commit in the dev branch that makes Tab Center aware of the Tab.hidden attribute. I didn't add any sort of "Hide tab" button, so you'll need another extension such as Conex to hide them for you.

I went for the optimized implementation, which means that I had to refactor the very core of the extension (tabs positioning), but that change was long due anyway and it should bring performance improvements. However, I'm pretty concerned about potential regressions from this patch, which is why I'd like you to play with it it 😀 Here are the things to test in particular:

So far I ironed out everything that I could see, but I might have missed stuff. Thanks!

gee-forr commented 6 years ago

@eoger - thank you so much for this! How would I go about building this extension to use locally on my machine?

eoger commented 6 years ago

There are no "building" steps anymore as we are using ES6 modules now 🎉

gee-forr commented 6 years ago

Awesome - thanks. I'm using it as we speak. And so far so good. I'll report back on this thread if I find any issues.

gee-forr commented 6 years ago

Hi,

Some quick feedback - I've been using it for about 12 hours with about 150 tabs across 8 tab groups, and everything is going great. Haven't noticed any issues.

Nice work, and thank you so much once again, I'm finally getting Firefox back to my pre-57 state. This extension + conex plays a huge part in that. I really appreciate the work you've done.

eoger commented 6 years ago

Thanks @gee-forr, appreciated!

I found a regression: Start state: Tabs [A, B, C, D]. Close tab B. Switch to tab C and Ctrl+Click a link. The new Tab will open in the wrong position. I think this is because I'm not shifting the index numbers in _remove(). I'll fix that when I get some time.

Keith94 commented 6 years ago

After quick test:

eoger commented 6 years ago

@Keith94 I wonder if you have these issues because the order was messed up earlier by the bug I describe in https://github.com/eoger/tabcenter-redux/issues/296#issuecomment-372512301. Can you still reproduce when "re-syncing" the tabs state by reloading the temporary extension?

Keith94 commented 6 years ago

I can still reproduce. Downloaded the dev build today. Also hit "reload" in about:debugging.

eoger commented 6 years ago

I couldn't reproduce the "Close Tabs underneath" problem.

Keith94 commented 6 years ago

nvm

eoger commented 6 years ago

Apologies for the delay, I've been pretty busy lately. Thanks a lot for the reports. The dev branch has been updated with the work I've been doing these past days. The good news is that I am way more confident now regarding potential regressions, because we finally have tests! Would you mind giving it a try again? (see https://github.com/eoger/tabcenter-redux/issues/296#issuecomment-372127510 for instructions).

gee-forr commented 6 years ago

Excellent news! I'm running the latest revision right now :) I'll post some feedback if I find any issues.

gee-forr commented 6 years ago

Hi @eoger - I've been running this version for a day or two now, and haven't noticed any of the issues from the previous version with tabs being shown out of order. This version looks to be very well put together - thanks :)

Keith94 commented 6 years ago

@eoger The previous issue seems to have been fixed.

Another thing, the throbber animations are no longer synced with each other now.

eoger commented 6 years ago

Merged in https://github.com/eoger/tabcenter-redux/commit/ef7b368ca596c7bb61dcc967508c9212af35076d, thanks everyone!
This work demanded a big/risky refactor, but it was definitely worth the trouble since it will allow us to make other improvements that were not even possible or very hard to implement in the past.

Feel free to open new issues if you notice something wrong once it goes live.

gee-forr commented 6 years ago

Fantastic news! When will you release to AOM?