Skip to content
This repository has been archived by the owner on Apr 1, 2020. It is now read-only.

Close tabs by middle clicking #2069

Closed
AUTplayed opened this issue Apr 7, 2018 · 4 comments
Closed

Close tabs by middle clicking #2069

AUTplayed opened this issue Apr 7, 2018 · 4 comments

Comments

@AUTplayed
Copy link

Would love to be able to close editor tabs by clicking mouse3 (middle click, mouse wheel click...) like in many other programs.

@texhnolyze
Copy link
Contributor

I am currently having a look at this and did a first shot implementation of this, by adding a mousedown listener to both the tabs name and file icon (similar to the click listener to switch tabs).

Check https://github.com/onivim/oni/compare/master...texhnolyze:middle-click-tab-close?expand=1

I am however not very happy with this solution as it stands and would appreciate some suggestions:

  • One option would be to combine the onClick and onMouseDown event handlers and select/close tabs based on the React.MouseEvent.button value (which I already do for the middle click)
  • Another options if something like that is achievable nicely with react (I come more from an angular background) would be to write a general purpose middle click event handler to run any given function on a middle click of an element e.g.
<div  className="name"
         onClick={this.props.onClickName}
         onMiddleClick={this.props.onMiddleClick}>

On another note I noticed, that tabs do not close if you only have a single one which suprised me but makes sense from a neovim perspective. Should I track that in an issue?

@akinsho
Copy link
Member

akinsho commented May 7, 2018

@texhnolyze I think from a react perspective option one if much more preferable to option two especially as we have a few issues already which are caused by very general event handlers occasionally swallowing or obscuring events. I think having an on click that also listens for a middle click would be a good solution.

Re. option one Tbh never used middle click or listened for it so can't advise much there re the actual event, but I wonder if an onclick is triggered by middle click then could you check to see if the click is a middle click and if so run the function to close the tab?

re the tabs not closing if you only have a single one the reason for that is that an issue was raised a while back that if oni had only one tab remaining and it was closed neovim/oni would close which is how neovim responds to that so what is done instead now is a no name buffer is created on purpose to prevent that behaviour

@texhnolyze
Copy link
Contributor

texhnolyze commented May 9, 2018

@Akin909 I did an implementation of an onMouseDown event handler to handle both the click and middle click as sadly onClick only gets triggered when using left mouse button. So I have to check the events button value for left and middle click to prevent unexpected behaviour on right clicks. (see: b96cbc4)

Concerning the tabs from what I gather from your response is, that when I have a single tab of let's say resume.md and I close it, it should be closed but a new empty buffer with the oni tab name [No Name] should be opened? Did I get that correctly?
Because what is happening for me is that, when I try to close resume.md there is just nothing happening, which led me to assume I did not click the close button correctly and clicking it again the first time I encountered it.

Also concerning testing, I tried setting up a <Tabs /> ui-test, which actually tests clicks and middle clicks, but have some issues with it. When trying to mount a <Tabs> element in a test (so that the child elements are fully rendered as well), what happens is that I run into issues due to the <FileIcon> element in the tab which uses icon themes configured under "ui.iconTheme and is thus asynchronous, leading to me trying to access members on _icons, which is null. (see: https://github.com/texhnolyze/oni/blob/b96cbc41319ba4376690f2fb7031ffddb1d7d541/browser/src/UI/components/Tabs.tsx#L172,176)
Any suggestions on how to solve this testing issue?

bryphe pushed a commit that referenced this issue May 16, 2018
* Add middle click tab closing (#2069)

by adding a onMouseDown directive to both tab file icon and
tab name and checking the resulting React.MouseEvent for a
middle click

* Fix unused imports and whitespace in Tabs ui test (#2069)

* Update Tabs component test snapshot (#2069)

* Fix Tabs ui test to correctly retrieve children (#2069)

* Differentiate between single und multiple tabs in test (#2069)

* Use mousedown event for tab selection and closing (#2069)

by middle clicking and let the tab itself handle the logic
for it by checking React.MouseEvent.button value

* Update classnames dependency to lastest master branch

and write own @types module declaration so that it can be used in
testing nstead of using a mock. The mock does not suffice as the
way the actual module previously had to be imported was
"import * as classNames" where classNames was the actual function.

It is not possible to build a module in typescript/es6 imports, which
will directly return a function in the same way the dependency did
in commonjs module syntax. Instead when defining a function to be
returned as default export it is returned as an Object like this
"{ default: [Function] }", which is correctly resolved when importing
with "import classNames from 'classnames'".

This only previously worked in production as webpacks ts-loader,
handles this issue, whereas when testing the sources are only compiled
with tsc.

There is an update to the classnames dependency on the current master,
but there hasn't been a release since 2006. So options were to setup
webpack for tests as well or add updated classnames dependency which
sets a "default" value on its commonjs exports.

Links for reference:
JedWatson/classnames#152
JedWatson/classnames#106
DefinitelyTyped/DefinitelyTyped#25206
microsoft/TypeScript#2719

* Fix tab click onMouseDown callback

* Test tab clicks to select/close trigger callbacks

* Mock child react components directly to test smaller unit

* Reset calls to callback mocks in each test case

* Add tests for tabs interaction with FileIcon/Sneakable
@keforbes
Copy link
Collaborator

#2190 has been merged in so I think this issue is resolved. Closing.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

No branches or pull requests

4 participants