Quicksaver / Tab-Groups

Reimplementation of Firefox Tab Groups as an add-on.
https://addons.mozilla.org/firefox/addon/tab-groups-panorama/
Mozilla Public License 2.0
545 stars 73 forks source link

Enter group view when closing last tab #354

Closed mzso closed 7 years ago

mzso commented 8 years ago

Hi!

I could swear that this used to work this way. But now I it doesn't. Now I just keep getting an empty tab opened. I don't know what's changed. I don't see why Tab Mix Plus would have changed anything in this regard.

Quicksaver commented 8 years ago

This was an intentional change, but you can revert it. Uncheck "Automatically close empty groups" in the add-on's preferences.

Quicksaver commented 8 years ago

Or rather, check it. Please reopen if it is checked and the group's still aren't closing. Note that groups with a name or any catch rules or options different from the defaults are not considered empty, so they are never closed automatically.

mzso commented 8 years ago

That option does not apply for normal view. When I close the last tab I get a new tab opened, never even enter panoramam mode. I remember that it used to enter panorama mode. Maybe Tab Mix Plus did change something.

Quicksaver commented 8 years ago

Ah yes, close the last tab and a new tab opens. Close that new tab and it will take you to groups view. ;)

It's a new feature to try to adapt to as many different use cases as possible: i) if you want to close all tabs in the group but don't want to actually leave it; ii) if you want to get rid of the group entirely all it takes is just one more action.

mzso commented 8 years ago

Nope. I get a new tab opened every time. I went through TMP's settings in the closing last tab section, but I never get group view, only if I manually enter. (I have Tab Mix Plus 0.5.0.1pre.160625a1 )

Quicksaver commented 8 years ago

This shouldn't have anything to do with TMP. Does it work if you disable all your other add-ons except for Tab Groups (TMP as well)?

mzso commented 8 years ago

@Quicksaver commented on 2016. jún. 27. 21:38 CEST:

This shouldn't have anything to do with TMP. Does it work if you disable all your other add-ons except for Tab Groups (TMP as well)?

How come?? The TMP has settings explicitly for tab closing behavior. I was quessing that it opens a new tab before TG can react to it.

I isntall TG in my test acccount. (Which doesn't have TMP only few other addons.) It worked the way you described it. I think this behavior is counter intuitive. You get a new tab opened when you close the last. I think closing the tab and entering panorama mode to show the empty group is more intuitive.

Quicksaver commented 8 years ago

How come?? The TMP has settings explicitly for tab closing behavior. I was quessing that it opens a new tab before TG can react to it.

But TMP doesn't account for groups, which is not a feature of itself but of another add-on (this one).

I think this behavior is counter intuitive. You get a new tab opened when you close the last. I think closing the tab and entering panorama mode to show the empty group is more intuitive.

This could possibly be made optional through a preference. But how do you suggest this would act with TMP's tab closing options? I don't remember any that would even conflict, although I may be missing something (I'll have to look at those options in detail to be sure).

Quicksaver commented 7 years ago

I should note, I haven't been able to reproduce this behavior at all, regardless of my TMP settings. Could you give beta version 2.1b2 and see if you still have this issue? If you do, could you export your TMP's settings and link them here please?

mzso commented 7 years ago

No change. I still get an empty tab no matter how many times I close it. Anyway I exported my tmp settings, maybe it's of some use: TMPpref-2016-10-15.txt

Quicksaver commented 7 years ago

Ok, found it, it's the "Load on New Tab" setting not being set to the new tab. Let me see what I can do about that.

Quicksaver commented 7 years ago

Looking at that feature, and at how I believe it should be acting, I'm not sure I should override it in Tab Groups in any way.

Leaving the default value (new tab page) is a no brainer, everything works, so we work too.

Defining any other value (homepage, current page, duplicate page, custom address) is also a no brainer, if you set those then you want those to open. Tab Groups shouldn't interfere at all there, so it shouldn't take you to groups view there.

The blank page... That's the tricky one. On the one hand, it's mostly a placeholder tab, if you keep closing it then you likely don't want it. On the other hand, if you set it then you obviously do want it. I'm not entirely sure what's "expected" in this case. You set that value, what are your thoughts? What would you expect to happen? I'm inclined to not do anything and treat it as any other specifically set preference, if you set it then you want it so Tab Groups shouldn't interfere either.

mzso commented 7 years ago

@Quicksaver I don't see how that setting is relevant. The applies to opening a new tab and not closing the last tab.

But in case I'm not opening anything, I'm closing the last tab of the group. What I expect to happen is that tab closing and no new tab opening endlessly, which is supposed to happen with the last tab in a window, to prevent it from closing.

I assumed that the issue is an bad interaction with "Do not close window when closing last tab", which is for closing the very last tab of a window.

Quicksaver commented 7 years ago

I don't see how that setting is relevant.

The default Tab Groups behavior is to call the browser's open new tab command when you close the last tab. This command is overriden by TMP by whatever you set in that preference, which is why you get blank tabs.

What I expect to happen is that tab closing and no new tab opening endlessly

Right, and normally this would happen, except you set TMP to override the new tab page with a blank tab. Tab Groups is looking for the new tab page, so it's not finding it.

I assumed that the issue is an bad interaction with "Do not close window when closing last tab", which is for closing the very last tab of a window.

That option assumes that there are no hidden tabs, and it only makes sense that way, which may not be the case because you have hidden groups. So with groups that option is nulled and it's completely ignored.

But I believe the best solution will be this: if you close the last tab and you set TMP to override the new tab page with any option other than the default, it should take you directly to groups view. If I understood you correctly, it's what you expect to happen as well. Let me know if not.

mzso commented 7 years ago

This command is overriden by TMP by whatever you set in that preference, which is why you get blank tabs.

But why does TG do that? There's no need to open a new tab when you close the last tab of a group. Just enter groups view and show the group as empty. What tab opens is not the issue. It's that it opened in the first place.

Right, and normally this would happen, except you set TMP to override the new tab page with a blank tab. Tab Groups is looking for the new tab page, so it's not finding it.

??? Why does it need to look for it?

So with groups that option is nulled and it's completely ignored.

By TMP itself? If that's true it's both good and bad. It's necessary for the window to not be closed if every (hidden or not) tab is closed. Doesn't it activate in that case?

But I believe the best solution will be this: if you close the last tab and you set TMP to override the new tab page with any option other than the default, it should take you directly to groups view. If I understood you correctly, it's what you expect to happen as well. Let me know if not.

I don't think there needs to be a condition. Whatever the new tab page is, it should go into groups view so that you can choose where to go, what to do next. (Close the group, open a new tab in it, go to a different group, etc)

So in summary, if I understand correctly: TG causes this by opening a new tab for some reason(why?). Then searches for newtab for some other reason(why? what would it do with it). And when fails to find it this happens.

Quicksaver commented 7 years ago

But why does TG do that? There's no need to open a new tab when you close the last tab of a group. Just enter groups view and show the group as empty. What tab opens is not the issue. It's that it opened in the first place.

Closing the last tab in a group may not mean you're done with it, it just means you don't want that tab anymore. What if you close its last tab just to clean things up but want to remain in the same group? You'd have to go to groups view (and possibly recreate the group if it didn't have a name, which would close it when closing its last tab) just to go back again.

That's what this behavior tries to fix. See https://github.com/Quicksaver/Tab-Groups/issues/354#issuecomment-228835451

By TMP itself? If that's true it's both good and bad. It's necessary for the window to not be closed if every (hidden or not) tab is closed. Doesn't it activate in that case?

No, by Tab Groups. The definition of "last tab" is not always apparent or obvious when considering groups. I'd rather avoid inconsistencies in behavior (you may not immediately remember if you have other groups when you're trying to close the last tab or not) than try to follow every single possible scenario properly; something which would be only apparent to the add-on itself and not to the average user who would have no clue why sometimes closing the last tab does one thing and other times another.

Simply put, with Tab Groups you're seeing a group of tabs in the window, if you close its last tab (and the new tab that opens), you're done with the group not the window. Close the window if you're done with the window, close the tabs if you're done with the group.

What I meant with that option is irrelevant is, it only really makes sense when the window is the only "group", which is just not true when considering groups, even if you only have one.

mzso commented 7 years ago

What if you close its last tab just to clean things up but want to remain in the same group? You'd have to go to groups view (and possibly recreate the group if it didn't have a name, which would close it when closing its last tab) just to go back again.

That not in conflict with what I said. If you close the last tab, there's nothing to show so going to group view is the appropriate thing to do. Also the group should not be automatically closed, in any case. (unless there's a setting to do so). Empty groups do exist when I open a new one or close all tabs of one in groups view. If I'm not done with the group, I can just click the plus icon to open a new one.

No, by Tab Groups. The definition of "last tab" is not always apparent or obvious when considering groups. I'd rather avoid inconsistencies in behavior (you may not immediately remember if you have other groups when you're trying to close the last tab or not)

Well, I see no particular point in doing this. It's only activated when there's only a single tab. (I believe TMP doesn't care if the tabs are hidden or not) But I guess it's fine if you make sure that the window is not unexpectedly closed if you close the absolute last tab.

mzso commented 7 years ago

Also with the current behavior users might easily forget about whatever tabs they have in other groups. Or even that they have tab groups. Especially if they have TMP because this behavior is the exact match of what TMP does without groups. Which is not a good idea in my opinion.

Quicksaver commented 7 years ago

If you close the last tab, there's nothing to show so going to group view is the appropriate thing to do.

You may want that, but there are users who don't and prefer to stay within the group and starting over with a new tab. Hence me trying to please everyone. ;)

Also the group should not be automatically closed, in any case.

That depends on the preference "Automatically close empty groups" in the add-on's options.

Quicksaver commented 7 years ago

Or even that they have tab groups.

Well there's only so much the add-on can do... ;)

mzso commented 7 years ago

@Quicksaver commented on 2016. okt. 16. 14:43 CEST:

If you close the last tab, there's nothing to show so going to group view is the appropriate thing to do.

You may want that, but there are users who don't and prefer to stay within the group and starting over with a new tab. Hence me trying to please everyone. ;)

Well it doesn't hurt them if they need to press a plus button to open a new tab. A reminder of the other groups, by going into groups view wouldn't either.

That depends on the preference "Automatically close empty groups" in the add-on's options.

Well you could either bypass it for this case or. As you do it in groups view or just show the undow stuff as normal.

By the way you claimed you didn't want inconsistencies in behavior. But this is exactly that, if you think of this option. Because it's not closed.

Maybe you could add a setting as a compromise. Something like "Stay in normal view when last tab of group is closed" and have it disabled by default.

Well there's only so much the add-on can do... ;)

Well, if you show them in cases where it makes sense, such as this it would remind them.

Quicksaver commented 7 years ago

Well it doesn't hurt them if they need to press a plus button to open a new tab.

It's not "I want a new tab." It's "I don't want these tabs anymore. But do I want the same group or a new one?"

A reminder of the other groups, by going into groups view wouldn't either.

It would since entering groups view is a complete overhaul of your window, which means it's a possible break in your workflow if you don't actually need to go there. Not to mention the performance toll it can take when toggling in and out of groups, for as much as I try to optimize it, it always takes a toll, so the best optimization possible is actually not to do it at all when it's not needed.

mzso commented 7 years ago

Well, ultimately it's your choice. I don't find the current behavior practical.

Quicksaver commented 7 years ago

I don't find the current behavior practical.

To confirm, what do you mean by this, opening blank tabs ad infinitum (which of course it isn't and that's what I'm changing) or the behavior I described in https://github.com/Quicksaver/Tab-Groups/issues/354#issuecomment-228835451?

mzso commented 7 years ago

@Quicksaver commented on 2016. okt. 16. 18:38 CEST:

I don't find the current behavior practical.

To confirm, what do you mean by this, opening blank tabs ad infinitum (which of course it isn't and that's what I'm changing) or the behavior I described in #354 (comment)?

I meant the current behavior. To be honest I completely forgot about that comment. I guess it's better. Though I don't know how many people will try to close that new tab. We'll see.

Quicksaver commented 7 years ago

At the time I added this from a request (which I can't find the issue right now), so there's at least one person that likes it. :)

Quicksaver commented 7 years ago

Beta version 2.1b4 should behave as you expect now, please give it a try and let me know how it works for you.

mzso commented 7 years ago

@Quicksaver commented on 2016. okt. 17. 18:17 CEST:

Beta version 2.1b4 should behave as you expect now, please give it a try and let me know how it works for you.

Cool. So far so good. No issues.

Quicksaver commented 7 years ago

Great! Thanks for letting me know about this, let me know if it starts misbehaving like that again.