Open KerfuffleV2 opened 2 years ago
KerfuffleV2
commented
2 years ago If anyone is interested in the status of my feature pulls, the image below shows:
You also can't see it, but it's possible to middle click tabs/bookmarks to close them. (#216)
josh-berry
commented
2 years ago Wanted to ACK this and say sorry I haven’t properly responded yet. I have half a reply drafted but real life has been busy. Happy to take PRs on all of the above, though a few of those we might want to discuss in more detail just to make sure we agree on the desired user experience.
Thanks a lot for all your work so far, and I’ll try to get you a proper reply soon!
KerfuffleV2
commented
2 years ago @josh-berry Thanks for the reply! Glad to hear those are features you'd generally be interested in adding to Tab Stash. I hope the pulls/issues I created aren't causing you to feel pressured.
I'd be happy to discuss the changes in more detail — just let me know how you want to approach that. It might be helpful to have a brief real-time chat.
Regarding user experience: I'm generally aiming for something similar to existing (vertical) tab addons. For example, Sidebery and Tab Center Reborn. I'm not sure if you're familiar with those.
josh-berry
commented
2 years ago Is becoming a replacement for other tab extensions like Sidebery, Tab Center Reborn within the scope of your vision for Tab Stash?
OK, so I've been thinking on this question for a bit and I think the short answer is, "yes, sort of". I'm definitely interested in going in that direction, but I think Tab Stash is different from other tab-management extensions in two important ways:
Before I built Tab Stash, I had tried a bunch of other extensions (e.g. Tree Style Tab--which Sidebery is a fork of--and OneTab), and even native tab-grouping features from browsers like Vivaldi and Edge. (Well, Tab Stash predates modern Edge, but I've tried Edge since then.) I always found that I would wind up with a bunch of tab groups, and they would quickly grow disorganized because I would (a) not keep up with grouping things at all, and/or (b) wind up with tabs in the wrong groups because I would duplicate/open tabs on the same site as another tab in the group, but for a task unrelated to the group. (e.g. grabbing a convenient GitHub tab because I wanted to search for something on GitHub)
The thing I learned from this is that placing a tab into a group really should be an intentional act--tolerating a little extra mess in the set of open tabs in the browser actually leads to a more-organized system overall, because you have to intentionally file things in the right places.
And, of course, you have to make it as easy as possible to file/re-file things so that once you do wind up in a situation where you have a lot of open/disorganized things to sort through, you can sort them as quickly and efficiently as possible.
So at least for now (I'm sure this will have to be refined over time), there are a few "value statements" for Tab Stash that have emerged (and bear with me; this is the first time I'm writing them down):
I realize the above is getting a bit abstract, but hopefully it helps to explain my thinking a bit more. Let me know your thoughts!
josh-berry
commented
2 years ago Some more detailed thoughts on the items without PRs (for the ones with PRs, we can discuss there):
- The default context menu for links is shown when right clicking a tab or bookmark in the Tab Stash interface, which makes another tab management addon necessary to perform functions like unloading tabs, moving them to a new window, etc.
Agree this could be useful. There are some things in the "default" context menu that would be useful to keep, though. If we go down the path of replacing the context menu entirely (as opposed to adding a "..." menu, for example), things like "Open in..." and "Copy Link" should probably also be there.
- Tab Stash only seems to show tabs in the list after they've loaded, which means doing something like opening a link in the background can take a long time to show a visual indication that something actually happened.
Yup, agree this can be improved. (I even have a little spinner animation using the Tab Stash toolbar-button colors that could be used for this--it's the one used on the deleted-items page right now.)
- Pinned tabs are not displayed at all.
I'm honestly not sure how to deal with pinned tabs, though I agree in principle they could be shown in the UI.
Visually, I think I like the "Edge-style" approach (just show icons in their own row), but the "Vivaldi-style" approach (similar styling as to un-pinned tabs, but put them in a separate group/section) also makes sense because it does make it easier to see the titles and manage pinned tabs without resorting to a context menu. Or maybe there's another approach I haven't considered.
Behaviorally, there are also some questions to be considered--do pinned tabs get saved to the stash on a "stash all" operation? Do we need to have a way to remember the "pinned" status of a tab? What options do we want to offer users / where might reasonable people disagree on how pinned tabs should look and behave?
KerfuffleV2
commented
2 years ago @josh-berry
Thanks once more for the reply!
I realize the above is getting a bit abstract, but hopefully it helps to explain my thinking a bit more. Let me know your thoughts!
That all seems reasonable to me. The changes I'm interested are mostly just to provide functionality that currently requires another tab addon (unloading tabs, move a tab to another window, etc) or to display information that currently isn't visible without using another tab addon (like what container exists.)
I don't see any conflict between those goals and the changes I'm proposing.
There are some things in the "default" context menu that would be useful to keep, though.
Absolutely. I wouldn't want to do anything that removes existing functionality. I'm mostly just talking about adding context-relevant actions depending on what is right clicked so it would be possible to do things like unload tabs from the Tab Stash list.
Yup, agree this can be improved. (I even have a little spinner animation using the Tab Stash toolbar-button colors that could be used for this--it's the one used on the deleted-items page right now.)
I actually have a pull request that adds a loading indicator, but the point in this issue you responded to is about Tab Stash being slow to actually show the tab in its interface at all — loading indicator or not. I actually spent a pretty good amount of time trying to figure out what was going on but was unsuccessful.
I'm honestly not sure how to deal with pinned tabs, though I agree in principle they could be shown in the UI.
I personally don't use pinned tabs at all so I don't have a strong opinion there, but it does seem like showing them in some form would be necessary to completely switch to using Tab Stash.
I'd say there's a MVP approach which just gives people the option of seeing them even if it's not ideal. I'd say something like having an option (which could be disabled by default) to just show them in the normal tab list (at the top of the list, above other non-pinned tabs) would be reasonable. It may not be optimal but that could be iterated on: just providing some way to see and deal with them would be a big step forward.
On a slightly different topic: Would it help at all to publish a branch on my fork with all these changes merged in, so you could try that version and see how the changes feel? I've personally been dogfooding this change set with no issues.
Also, what is your stance on forking Tab Stash? Obviously it's open source so I could just go ahead and do it, but the impression I got from the README and other material is that's something you'd prefer to avoid. I'd prefer to avoid it too, but I also don't want my work to go to waste and there are people who seem like they'd benefit from these features being available as well.
An alternative to forking might be if you created an experimental version where merging in changes could be more permissive and people who needed them could use/test it without the normal expectation of stability that should exist for the main Tab Stash addon. Call it "Tab Stash Experimental" or something like that, and you'd still have control of the project. Once a feature had been refined and proven then it could be merged into Tab Stash. Just throwing that out there.
josh-berry
commented
2 years ago I don't see any conflict between those goals and the changes I'm proposing.
I agree!
Yup, agree this can be improved. (I even have a little spinner animation using the Tab Stash toolbar-button colors that could be used for this--it's the one used on the deleted-items page right now.)
I actually have a pull request that adds a loading indicator, but the point in this issue you responded to is about Tab Stash being slow to actually show the tab in its interface at all — loading indicator or not. I actually spent a pretty good amount of time trying to figure out what was going on but was unsuccessful.
I think they're actually being hidden/excluded in the UI itself as not being stashable. Probably this could be addressed by handling the "stashable/unstashable" distinction at lower levels--e.g. in the tab.vue component itself, and/or in the model when a user tries to perform some action like drag-and-drop on an unstashable tab.
I personally don't use pinned tabs at all so I don't have a strong opinion there, but it does seem like showing them in some form would be necessary to completely switch to using Tab Stash.
I'd say there's a MVP approach which just gives people the option of seeing them even if it's not ideal. I'd say something like having an option (which could be disabled by default) to just show them in the normal tab list (at the top of the list, above other non-pinned tabs) would be reasonable. It may not be optimal but that could be iterated on: just providing some way to see and deal with them would be a big step forward.
Yeah, this might be a good candidate for an experimental feature (like the popup view and "Open Tabs" experiments). In fact, it could probably be tied to the "Open Tabs" experiment itself.
On a slightly different topic: Would it help at all to publish a branch on my fork with all these changes merged in, so you could try that version and see how the changes feel? I've personally been dogfooding this change set with no issues.
Honestly, probably not--I try to do things incrementally, so having a larger number of smaller branches is actually easier for me to process. It also makes it a little easier for me to test because I can see exactly what in the code has changed, and focus on just those things. Reviewing/merging a large branch will probably take me longer than reviewing several smaller branches, even though the total set of changes is the same.
Also, what is your stance on forking Tab Stash? Obviously it's open source so I could just go ahead and do it, but the impression I got from the README and other material is that's something you'd prefer to avoid. I'd prefer to avoid it too, but I also don't want my work to go to waste and there are people who seem like they'd benefit from these features being available as well.
Yup, agree it's something I'd prefer to avoid, but of course since Tab Stash is open source, you can. That said, if we ever get to a point where there are differing over-arching goals for the project, then a fork might be a good thing.
But, at least from my side, I don't see a a reason for it right now--I think we're on the same page about everything you've proposed. 🙂
An alternative to forking might be if you created an experimental version where merging in changes could be more permissive and people who needed them could use/test it without the normal expectation of stability that should exist for the main Tab Stash addon. Call it "Tab Stash Experimental" or something like that, and you'd still have control of the project. Once a feature had been refined and proven then it could be merged into Tab Stash. Just throwing that out there.
So the approach I've gone with in the past--when I wasn't sure about a particular feature/implementation--was to create an "experiment" / feature flag that users can turn on if they choose. There are a few benefits to this over forking or branching:
Of course, it comes with some challenges--the new feature has to be somewhat isolated from the rest of the code, so the boundary between "production" and "experiment" is clear. Otherwise the complexity of the code can increase quite dramatically. And more caution is needed around code quality--experimental things shouldn't break non-experimental things.
KerfuffleV2
commented
2 years ago @josh-berry
I think they're actually being hidden/excluded in the UI itself as not being stashable.
Thanks, that's helpful! I will look into that part again when I get some free time.
[re: pinned tabs] In fact, it could probably be tied to the "Open Tabs" experiment itself
I'd probably go with just making it a separate experiment unless you're opposed to that since it's more flexible.
so having a larger number of smaller branches is actually easier for me to process.
Sorry if I wasn't clear, I didn't mean doing that instead of the separate PRs. I just meant so that you could build a version of the module to test in your actual browser without having to manually merge the branches together. Right now git can't merge them all automatically, so the merge process requires some manual resolving of conflicts.
But, at least from my side, I don't see a a reason for it right now--I think we're on the same page about everything you've proposed.
I agree. Time looks like the main thing that would lead to me going for the fork option at the moment. It's only been 10 days, but if my PRs are still in limbo after a month then that's about when I'll be looking at options like forking. I don't mean for that to sound like an ultimatum, or like I feel that I'm entitled to a response but I do want to be transparent about my plans.
And if I do end up forking at some point, I'd be happy to have the fork made obsolete if the changes eventually got merged. I don't have any desire to replace Tab Stash with my own extension.
create an "experiment" / feature flag that users can turn on if they choose.
I agree that's a good option for letting users opt into stuff that causes a visible effect and might break a user's workflow (or just not be what they want.) A limitation is that the option can control a visible effect but it can't necessarily help with stability of new internal changes. Just for example, #221 could have an option to disable the tooltips/indication on folders with open tabs but internally there are changes to aggregate folder stats and if there are any issues with that code, simply turning off the option won't really help. I don't think there's really a practical way to make it so no new/changed code runs at all if the option is disabled in this case.
For that sort of thing, a separate "unstable" version of Tab Stash could work better for letting users preview possible new features/test the code without stability problems impacting the perceived reliability of Tab Stash itself, if that makes any sense. It would also reduce the work required for evaluating PRs because the standards wouldn't need to be as high.
josh-berry
commented
2 years ago @KerfuffleV2
[re: pinned tabs] In fact, it could probably be tied to the "Open Tabs" experiment itself
I'd probably go with just making it a separate experiment unless you're opposed to that since it's more flexible.
Fine by me!
so having a larger number of smaller branches is actually easier for me to process.
Sorry if I wasn't clear, I didn't mean doing that instead of the separate PRs. I just meant so that you could build a version of the module to test in your actual browser without having to manually merge the branches together. Right now git can't merge them all automatically, so the merge process requires some manual resolving of conflicts.
Thanks, I understood--I tend to test incrementally as well as review incrementally. So while it would be a cool demo, I don't want to ask you to do some throw-away work when I'll want to do the testing and review on each PR individually anyway.
But, at least from my side, I don't see a a reason for it right now--I think we're on the same page about everything you've proposed.
I agree. Time looks like the main thing that would lead to me going for the fork option at the moment. ...
Yup, understood and thanks for being transparent. In the same spirit: my day job (and "real life" in general) can get demanding at times, and so how much time I have for Tab Stash will vary. I also try to prioritize bugs over features, so for example #214 took precedence over reviewing your PR.
I do try to be consistent in spending some amount of time week by week, but since there are 13k users and only one of me, it can be hard to keep up occasionally. 😄
create an "experiment" / feature flag that users can turn on if they choose.
I agree that's a good option for letting users opt into stuff that causes a visible effect and might break a user's workflow (or just not be what they want.) A limitation is that the option can control a visible effect but it can't necessarily help with stability of new internal changes. Just for example, #221 could have an option to disable the tooltips/indication on folders with open tabs but internally there are changes to aggregate folder stats and if there are any issues with that code, simply turning off the option won't really help. I don't think there's really a practical way to make it so no new/changed code runs at all if the option is disabled in this case.
Yes, that's true (and exactly what I was alluding to re. quality)--it definitely does require more time/effort in the short term. (Though this can be mitigated somewhat by ruthlessly sticking to building only an MVP and improving on it later.)
But let me step back a bit. What I'm trying to do is optimize for throughput rather than latency--given a fixed/limited amount of time to work on Tab Stash, deliver the most value over time (rather than delivering something as quickly as possible).
My own industry experience has taught me that smaller changes are disproportionately easier to test/review/verify than larger changes. (If you want to be pedantic, code review and testing has complexity worse than O(N), where N is the size of the change.) Hence my preference for more, smaller PRs.
An experimental branch does reduce reviewing/testing work in the short term, but increases it in the long term. This is because code moving from experimental -> master still has to meet a high quality standard. Except with an experimental branch, you either have to break apart what's in the branch for inclusion in master (after you have lost context on your changes, added bugfixes, mixed other features into the branch, etc.), or you have to do a big mega-merge, and then you're getting hit by that verification complexity. Then there's also the added overhead of releasing/maintaining/supporting the two branches.
So I try to avoid branches if at all possible (and even resisted creating a stable branch until relatively recently), just because it's overall lower-overhead to keep master as close to a shippable state as possible at all times.
Coming back to feature flags, I actually don't think anything you've proposed so far (apart from showing pinned tabs) even needs a branch/feature flag, since there's not much that reasonable people would disagree on [he says without having reviewed them in detail yet... 🤣]. I think the main use for feature flags is cases where we're not sure yet what would make for a great UX. "I'm not sure about how this should behave and I want feedback from actual users" => great candidate for a feature flag.
TL;DR: Let's go straight to master (and gate pinned tabs behind a feature flag, to get some actual user feedback). IMO branching is going to slow things down.
KerfuffleV2
commented
2 years ago So while it would be a cool demo, I don't want to ask you to do some throw-away work
No worries. It wouldn't be any extra work through, if you ever change your mind: I'm already maintaining a branch with the features merged for my own use. It's just not remote.
In the same spirit: my day job (and "real life" in general) can get demanding at times, and so how much time I have for Tab Stash will vary.
Absolutely understood.
TL;DR: Let's go straight to master
Makes sense! (And I did read the whole thing. :)
josh-berry
commented
2 years ago OK, so of the branches you've sent me so far, here's where we're at I think (making a note here for my own reference too):
(once I've gotten some sleep :) )
rayman89
commented
2 years ago This #273 as well will help as well for a better experience as a vertical tab extension.
Question
Is becoming a replacement for other tab extensions like Sidebery, Tab Center Reborn within the scope of your vision for Tab Stash? It's actually quite close to being able to replace other tab/vertical tab addons.
Incomplete list of what I see as important but currently missing functionality:
Features
Checked items have a pending pull request that adds the feature.
Merged