Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Configurable behavior for right click on the "New Tab" button #1751

Closed
Lej77 opened this issue Jan 31, 2018 · 22 comments
Closed

Comments

@Lej77
Copy link
Contributor

Lej77 commented Jan 31, 2018

Short description

I would like the ability to configure what a right click on the "New Tab" button does. Currently we have this ability for a middle click on the "New Tab" button and I would like a similar option for right clicks. I would suggest having the default behavior be to open a new tab as the next sibling of the current tab since there isn't any easy way to open a new tab in this position currently, at least not with the default settings.

Steps to reproduce

  1. Start Firefox with clean profile.
  2. Install TST.
  3. Right click on the "New Tab" button.

Expected result

Open a new tab as child/sibling/next sibling of current tab.

Actual result

Opens a tab in the same way as on left click of the "New Tab" button. Also shows the context menu for a second before it is automatically closed.

Environment

  • Platform (OS): Windows 10
  • Version of Firefox: 58
  • Version (or revision) of Tree Style Tab: 2.4.7
piroor added a commit that referenced this issue Feb 1, 2018
The action should open the context menu.
@piroor
Copy link
Owner

piroor commented Feb 1, 2018

Sorry, current result on Windows is unexpected. Truly expected result for right-click on the new tab button is opening context menu instead of new tab, and actually TST on Ubuntu doesn't open new tab for the action. e2bb0c5 should fix this "error".

I'm negative to provide extra configurable behavior for right-click, from some reasons:

  • I think that right-click should open context menu consistently, to avoid confusion of people who want to open context menu but get new tab accidentally.
  • Even if it become configurable, I won't use the feature (because I expect that right-click should open context menu always.) As the result, codes for configurable right-click on the new tab button will be left unmaintained for long time.

@piroor
Copy link
Owner

piroor commented Feb 1, 2018

@Lej77 How do you think about the consistency of right-click for the context menu? Even if the action didn't open new tab from initial, did you expected to open new tab by right-click on the button?

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 1, 2018

@piroor I tested Firefox behavior when right clicking the tab bar's "New Tab" button and that does just open the context menu so your probably right that right clicks should always open the context menu and nothing else.

I would still like a way to open a new tab as next sibling without losing the ability to open a new tab as child of current tab. Currently I either open a new tab as child of the current by middle clicking the "New Tab" button and then drag and drop it to its wanted position or I middle click the reload button and then left click a bookmark to the new tab URL. They both work but drag and drop takes a few seconds to do and duplicating the current site can also take a second if its a heavy site. Its not really a big issue but it is slightly inconvenient, can you think of another way to make this easier or should I just ignore it?

@piroor
Copy link
Owner

piroor commented Feb 1, 2018

Just an idea, how about a submenu like "add new tab at..." for the context menu? For example:

  • Reload Tab
  • Mute Tab
  • ...
  • Close Tab
  • Tree Style Tab >
    • Open a New Tab at... > (new)
      • as Child (new)
      • as Next Sibling (new)
    • Reload Tree
    • ...

Or, new submenu by an addon which is designed to work together with TST like:

  • Reload Tab
  • Mute Tab
  • ...
  • Close Tab
  • Tree Style Tab >
  • Open a New Tab at... > (new, added by another addon for this purpose)
    • as Child
    • as Next Sibling

@piroor
Copy link
Owner

piroor commented Feb 1, 2018

One more idea: a popup menu like container selector. Currently TST has a simple container selector to open new tab in specific container (Private, Work, etc.) at the right edge of the "New Tab" button. Then, I can add another selector opposite the container selector like:

[v     +     v]

When I click the right chevron:

     |Private |
     |Work    |
     |Banking |
     |Shopping|
[v     +    |v]

When I click the left chevron:

|Child Tab  |
|Sibling Tab|
[v|    +     v]

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 1, 2018

@piroor I think both of your suggestions could work. The context menu options have the advantage that they are easier to implement and to toggle on and off since you already have configurable context menu entries. On the other hand I think that your second idea with popup menu like the container selector would be faster to use. Which one is best probably depends on the person using it. I would prefer a popup menu like the container selector but everyone might not agree with me.

@piroor
Copy link
Owner

piroor commented Feb 6, 2018

The selector UI has been implemented by recent commits. By default it is shown by long-press on the new tab button. Moreover you can configure to show a small button to show the selector immediately.

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 6, 2018

@piroor I tested the latest TST Build (v2.4.8.6464) and I really like the new design. But I think I found a bug: if you set Long-press on the "New Tab" button to to choose container it still shows the same menu as when the specify where new tab is opened in alternative is selected. The (nothing) alternative works as expected though.

piroor added a commit that referenced this issue Feb 6, 2018
@piroor
Copy link
Owner

piroor commented Feb 6, 2018

Oops...

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 6, 2018

@piroor Seems fixed now! Another bug: right clicking one of the selectors cause it to open but it also opens the context menu. It should probably only do one of those things.

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 6, 2018

@piroor Actually the selector closes itself after you move the mouse. Still it shouldn't open to begin with.

Environment

  • Platform (OS): Windows 10
  • Version of Firefox: 58
  • Version (or revision) of Tree Style Tab: 2.4.8.6476

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 6, 2018

@piroor I'm trying to use the CSS code for tab-counts-in-new-tab-button-1661 with the open tab as selector and I wanted the tab count to slide away from the selector when it is shown. I was going to use the general sibling selector to detect when the selector is open (when the class open is added to it) but it only works when the target element is after the element that affects it. Is there a reason why the selectors are after the .newtab-button element or could they be before it to make the CSS code simpler?

To be clear, the question is if the current sidebar HTML code could be changed from:

<div class="after-tabs vbox">
    <div class="newtab-button-box vbox">
        <button class="newtab-button"
                title="__MSG_tabbar.newTabButton.tooltip__"></button>
        <button class="newtab-action-selector-anchor"
                title="__MSG_tabbar.newTabAction.tooltip__"
                data-menu-ui="newtab-action-selector"></button>
        <button class="contextual-identities-selector-anchor"
                title="__MSG_tabbar.newTabWithContexualIdentity.tooltip__"
                data-menu-ui="contextual-identities-selector"></button>
    </div>
</div>

to:

<div class="after-tabs vbox">
    <div class="newtab-button-box vbox">
        <button class="newtab-action-selector-anchor"
                title="__MSG_tabbar.newTabAction.tooltip__"
                data-menu-ui="newtab-action-selector"></button>
        <button class="contextual-identities-selector-anchor"
                title="__MSG_tabbar.newTabWithContexualIdentity.tooltip__"
                data-menu-ui="contextual-identities-selector"></button>
        <button class="newtab-button"
                title="__MSG_tabbar.newTabButton.tooltip__"></button>
    </div>
</div>

in order to improve custom CSS code.

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 6, 2018

Here is the CSS code I have written so far:

:root {
  --tab-number-newTabButton-margin-left: 0.4em;
  --tab-number-newTabButton-margin-top: 0.1em;
  --tab-number-newTabButton-color: var(--tab-text);
  --tab-number-newTabButton-background: transparent;
  --tab-number-newTabButton-text: 1em;
}

#tabbar {
  counter-reset: tabs;
}
.tab {
  counter-increment: tabs;
}

.newtab-button::after {
  content: counter(tabs) " tabs";
  pointer-events: none;
  
  background: var(--tab-number-newTabButton-background);
  color: var(--tab-number-newTabButton-color);
  
  position: absolute;
  left: 0px;
  top: 0px;

  padding-left: var(--tab-number-newTabButton-margin-left);
  padding-top: var(--tab-number-newTabButton-margin-top);
  font-size: var(--tab-number-newTabButton-text);
}
:root.newtab-action-selectable .newtab-button::after {
  transition: left 0.1s, opacity 0s 0.05s;
}
:root.newtab-action-selectable .newtab-button-box:hover .newtab-button::after {
  left: 20px;
  opacity: 1;
}
.newtab-button::after {
  opacity: var(--button-opacity);
}
.newtab-button:hover::after {
  opacity: 0.98;
}

This code works when the "New Tab" button is hovered but not when the open tab as selector is opened and the mouse hovers over an item in the opened menu. To fix this I would like to change:

:root.newtab-action-selectable .newtab-button-box:hover .newtab-button::after {
  left: 20px;
  opacity: 1;
}

to:

:root.newtab-action-selectable .newtab-action-selector-anchor.open ~ .newtab-button::after,
:root.newtab-action-selectable .newtab-button-box:hover .newtab-button::after {
  left: 20px;
  opacity: 1;
}

So that the tab count kept its offset while the menu is open. Unfortunately the ~ selector only works for elements placed before the target element. So I can't use it currently. This means that if I want my code above to work I have to use the .newtab-action-selector-anchor::after element for my tab count instead of the .newtab-button::after element. This makes it complicated though since I would have to make that element always visible and therefore can't use the fade in effect.


I modified the CSS code to work as intended using the .newtab-action-selector-anchor::after element instead:

:root {
  --tab-number-newTabButton-margin-left: 0.2em;
  --tab-number-newTabButton-margin-top: 0.15em;
  --tab-number-newTabButton-color: var(--tab-text);
  --tab-number-newTabButton-background: transparent;
  --tab-number-newTabButton-text: 1em;
}

#tabbar {
  counter-reset: tabs;
}
.tab {
  counter-increment: tabs;
}


.newtab-action-selector-anchor {
  opacity: 1 !important;
  overflow: visible !important;
}
:root:not(.newtab-action-selectable) .newtab-action-selector-anchor,
.newtab-button-box:not(:hover) .newtab-action-selector-anchor:not(.open) {
  border-right: none !important;
}
.newtab-action-selector-anchor::before {
  transition: var(--tab-animation);
}
:root:not(.newtab-action-selectable) .newtab-action-selector-anchor::before,
.newtab-button-box:not(:hover) .newtab-action-selector-anchor:not(.open)::before {
  opacity: 0;
}

.newtab-action-selector-anchor::after {
  content: counter(tabs) " tabs";
  white-space: nowrap;
  pointer-events: none;
  
  background: var(--tab-number-newTabButton-background);
  color: var(--tab-number-newTabButton-color);
  
  position: absolute;
  left: 0;
  top: 0;

  padding-left: var(--tab-number-newTabButton-margin-left);
  padding-top: var(--tab-number-newTabButton-margin-top);
  font-size: var(--tab-number-newTabButton-text);
}

:root.newtab-action-selectable .newtab-action-selector-anchor::after {
  transition: left 0.1s, opacity 0s 0.05s;
}
:root.newtab-action-selectable .newtab-action-selector-anchor.open::after,
:root.newtab-action-selectable .newtab-button-box:hover .newtab-action-selector-anchor::after {
  left: 20px;
  opacity: 1;
}
.newtab-action-selector-anchor::after {
  opacity: var(--button-opacity);
}
.newtab-button-box:hover .newtab-action-selector-anchor::after {
  opacity: 0.98;
}

Since it has to modify the selector it is a bit more complicated but it mostly works. The only problem is that the border doesn't fade in.

@piroor
Copy link
Owner

piroor commented Feb 7, 2018

OK, I've changed DOM structure as suggested at #1751 (comment)

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 7, 2018

@piroor Thank you!

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 7, 2018

@piroor Did you fix that right click opens the selector menus?

@piroor
Copy link
Owner

piroor commented Feb 7, 2018

While these selector UIs are shown contextmenu event is canceled, so you won't see context menu on such cases. On the other hand right-click on these selector UIs triggers the clicked command, this behavior respects the behavior of native menu (confirmed on Ubuntu).

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 7, 2018

@piroor That is not the behavior I observe. When I right click one of the selectors it shows both the selector menu and the context menu. However when I move the mouse the selector's menu closes itself. The context menu remains open though. This is on Windows 10, Firefox 58, TST v2.4.8.6481.

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 7, 2018

@piroor Also when I right click the "New Tab" button the context menu appears but when I move the mouse it closes itself.

@piroor
Copy link
Owner

piroor commented Feb 7, 2018

Sorry I misunderstood your comment at #1751 (comment) and I thought that it is about right-click on the selector menu, not on the anchor. By the commit 495fb91 now right-mouse-down on the down triangle button won't open the selector menu.

@piroor
Copy link
Owner

piroor commented Feb 7, 2018

And the problem "when I move the mouse the selector's menu closes itself" and "when I move the mouse it closes itself" are also fixed for me by recent commits.

@Lej77
Copy link
Contributor Author

Lej77 commented Feb 7, 2018

@piroor I tested TST v2.4.8.6483 and the context menu issues seems to be fixed for me as well. Also I posted my workaround for the tab counter when the new tab selector is used in #1661. That is the same issue as the wiki links to in tab-counts-in-new-tab-button-1661 so it should be easy for users to find.

@Lej77 Lej77 closed this as completed Feb 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants