Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Add package option to re-enable single-click behaviour #804

Closed
wants to merge 3 commits into from
Closed

Add package option to re-enable single-click behaviour #804

wants to merge 3 commits into from

Conversation

Alhadis
Copy link
Contributor

@Alhadis Alhadis commented Apr 13, 2016

Some users might prefer the old behaviour of single-clicking a file in the tree-view to open it. This PR adds a package setting (off by default) to restore the pre-1.7.0 method of selecting files.

Please let me know if a spec should be included as well. =)

@@ -65,6 +65,12 @@
"type": "boolean",
"default": true,
"description": "Focus the tree view when revealing entries."
},
"singleClickSelect": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

openFilesWithSingleClick conveys the intent clearer I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aye, agreed.

@50Wliu
Copy link
Contributor

50Wliu commented Apr 14, 2016

Yes, tests would be great.

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 14, 2016

On it.

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 14, 2016

@50Wliu Done! =)

@50Wliu
Copy link
Contributor

50Wliu commented Apr 14, 2016

/cc @kuychaco

@lee-dohm
Copy link
Contributor

Thanks @Alhadis for putting this together. I'm going to review it with the team because I'm concerned that just having a separate setting for this in tree-view will be confusing for people. For example, someone sees that tree-view.openFilesWithSingleClick is unchecked but files still open on single-click because core.allowPendingPaneItems is checked. This is kind of the same situation we have with the various types of ignored files and the separate settings between core and tree-view.

@lee-dohm lee-dohm self-assigned this Apr 14, 2016
@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 14, 2016

Hrm... perhaps roll them together into one multiple-choice option..?

  1. Single-click: Preview file
  2. Single-click: Open file
  3. Double-click: Open file

Some descriptive text underneath the menu could elaborate on what each mode does. =)

Just throwing thoughts out there, haha.

@lee-dohm
Copy link
Contributor

Yeah, I was thinking about something like that perhaps. That has generally been more understandable in the past than a bunch of checkboxes spread around. If anything, I would want there to be one setting that changes the behavior in all locations in a uniform way (tree-view, find-in-project results ... any place that can open a file).

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 14, 2016

Well, just let me know if I need to amend to my pull request, or anything. ^^

@bjmiller
Copy link

I think it's worth it to merge this change for now, and test it out in the beta channel, just to fix broken workflows for us as a quick-fix. A "nicer" way of handling this can supersede this, after appropriate discussion of the "right" way to do it.

@acontreras89
Copy link
Contributor

For example, someone sees that tree-view.openFilesWithSingleClick is unchecked but files still open on single-click because core.allowPendingPaneItems is checked.

Or the other way around: you won't be able to open files with a single click if you also want to enable pending items (keep in mind this is now a core functionality, not limited to the tree-view or tabs package anymore.)

I would rather not merge this until we have a comprehensive solution.

@bjmiller
Copy link

@acontreras89 With all due respect, it's bad form to break a feature that people take for granted, and then interest on a delay for a "comprehensive" solution. The proper choices here are either to restore the functionality quickly, or revert the change entirely - until you have a "comprehensive" solution.

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 15, 2016

Or the other way around: you won't be able to open files with a single click if you also want to enable pending items

@acontreras89 I think you should actually check the PR's modifications. Nothing is broken regarding the pending items feature.

Also, I completely back what @bjmiller said.

@acontreras89
Copy link
Contributor

@Alhadis maybe I got it wrong, but we're opening the file with pending: true if either core.allowPendingPaneItems or tree-view.openFilesWithSingleClick are set. In that case, when both flags are set we will get a pending item instead of a regular one when opening by single-click, won't we?

You could work around this with something like:

if atom.config.get('core.allowPendingPaneItems') or atom.config.get('tree-view.openFilesWithSingleClick')
  atom.workspace.open(entry.getPath(), pending: not atom.config.get('tree-view.openFilesWithSingleClick'), activatePane: atom.config.get('tree-view.openFilesWithSingleClick'))

@bjmiller, regarding whether or not this should be merged in its current state, I still think haste makes waste:

  • PRs have a cost for both the community and Atom's team. Who would take care of continuing this effort?
  • Adding, removing or modifying config options is a delicate thing to do and affects all users, so it shouldn't be done carelessly
  • Partial solutions or workarounds usually have an impact on maintainability, legibility, and/or performance

That's just my opinion, though.

I'd agree this should be merged if, for instance, it wasn't adding a config option and just ensuring the file gets open on single-click when core.allowPendingPaneItems is set to false (that would be "restoring the functionality quickly".)
Or if the time needed to discuss and implement the correct solution would prevent this change from taking place in the following releases (which I strongly believe is not the case here.)

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 15, 2016

@acontreras89 I think you did get it wrong. Passing pending: true does nothing if the user doesn't have the Allow Pending Items option enabled.

So the code you've suggested is functionally equivalent.

@acontreras89
Copy link
Contributor

acontreras89 commented Apr 15, 2016

Pending items are used by several packages (not exclusively tree-view.) I'm assuming one could want to use them (e.g.) with find-in-project but still open files by single-clicking them in tree view.

A user with core.allowPendingPaneItems set could be confused about tree-view.openFilesWithSingleClick setting, as switching it on or off makes no difference for him.

Edit:

So the code you've suggested is functionally equivalent.

Pending items Single-click open Original code Suggested change
Open pending item Open item
Open pending item Open pending item
Open item Open item
Do nothing Do nothing

And currently there's also inconsistency with the activatePane flag, which set (by default) when double-clicking but is overridden when single-clicking. I don't know if this is intentional, though.

P.S: I was just trying to get somewhere here, but seeing the reactions and replies to my comments, I'll just let it be. Whatever

@efatsi
Copy link

efatsi commented Apr 15, 2016

I'm a fan of the code @acontreras89 suggested above:

if atom.config.get('core.allowPendingPaneItems') or atom.config.get('tree-view.openFilesWithSingleClick')
  atom.workspace.open(entry.getPath(), pending: not atom.config.get('tree-view.openFilesWithSingleClick'), activatePane: atom.config.get('tree-view.openFilesWithSingleClick'))

Clears up confusion about having both items checked.

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 15, 2016

... what.

@acontreras89 Please give a cold, clear example of how this supposed problem would surface. Describe an example situation.

I'm still confused as hell as to what you're trying to put forward.

@ripter
Copy link

ripter commented Apr 18, 2016

What's the status on this? When can we expect the fix?

@ripter
Copy link

ripter commented Apr 18, 2016

I don't understand 2, how does this provide options for people? You took away an option, before we could just turn off Pending Pane (which was also forced on us without notice in an update.) But now that toggle does nothing useful. It just enables/disables using a pending pane for single click in the tree view. When it's off, single click no longer works. You forced everyone to always use double click. Atom is now the only app I use that requires double click and it's very disruptive.

I am willing to help with a plugin to restore the single click option. Or a fork of the TreeView to something that doesn't constantly change behavior with every update.

@bjmiller
Copy link

@LEEDOHM I'm sorry man, but I have to vent here.

First, don't say "other editors". Say what you mean: Sublime Text.

Sublime's default behavior is downright weird, and should NOT have been introduced into atom. No other editor behaves like this, and new users of ST generally find it a difficult UX to get used to.

Pending panes was a bad move, made even worse by altering more normal behavior for those of us who never cared for Sublime, and never wanted that feature.

Please don't be so focused on converting Sublime users that you replicate all of its bad parts, too.

@lee-dohm
Copy link
Contributor

I would have said Sublime Text if that is what I meant. We based the functionality on multiple editors, not only Sublime Text.

@ripter
Copy link

ripter commented Apr 18, 2016

One reason this is so frustrating is because there was a package to enable the Sublime behavior: https://github.com/mattsfrey/atom-double-click-tree-view
Then you made the double click default, but allowed the user to configure it. Now you've removed the ability to configure it.

You are telling people to add a line to their init.js/init.coffee file. Which just pushes the maintenance issues onto the users. Everyone that wants the normal behavior has to find that line, copy into their init.js, and hope to god that you don't change something else.

The original code does a better job at fulling your three bullet points than the new code. It worked as expected for new users, it was configurable, and it didn't add maintenance issues to either side.

@Dagrut
Copy link

Dagrut commented Apr 19, 2016

I don't know much about atom's structure and how it's working internally, so I may be wrong, but as far as I can see, this is how it should be : Tab preview should always be enabled (no option for it), and each plugins should have an option to use it or not if they need it.
This way, all the required options (including a single-click file opening) would be grouped in the tree view plugin, and would not cause any problem (For example the search plugin may have an option to enable tab preview without changing the behavior of the tree view plugin).
Would that be possible?

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 19, 2016

Provides options for people

Providing options by denying options. Okay, makes perfect sense.

@Alhadis Alhadis deleted the single-click-select branch April 19, 2016 07:55
@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 19, 2016

I'm just glad the workaround @olmokramer brought up works seamlessly.

However, I seriously disagree with this rationale - having the option to open files by single-clicking in the package is detached from the core settings, and would've existed as an easy fallback for users who prefer the "legacy" behaviour.

Since it would've been off by default, I'm puzzled as to how this could've posed an issue for new users accustomed to the behaviour of other editors.

Alhadis added a commit to Alhadis/.atom that referenced this pull request Apr 19, 2016
As discussed in atom/tree-view#804, the ability to open a file by single
clicking won't happen. On the other hand, fuck the pending item feature.
@olmokramer
Copy link
Contributor

Ok, I've wrapped my fix inside a package: single-click-open. In addition to opening files with a single click from the tree view, it also removes the double click on the search results in the project-find results view :)

@citylims
Copy link

thanks @olmokramer, and get a grip Atom team jeez

@jwoertink
Copy link

wow.. sad times 😢 For the life of me, I can't figure out why someone would want to single click on a file and have it do nothing. I stopped using textmate 2 because it was a double click to open a file. The most annoying thing though is updating, and having things just changed. Atom now feels clunky to me.

My suggestion is that when a change that changes UX happens, it should be a major release, and not a minor release. Going from 1.7 to 1.8 should be new features that can be enabled. Changing a developers workflow is a breaking UX change that should be a 2.0 release. (IMO).

@smaili
Copy link

smaili commented Apr 19, 2016

@jwoertink cheers to that 🍺

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 19, 2016

This goes beyond simple annoyance, though. This has actually hurt the trust I have in Atom as an editor.

There was no reason to change what wasn't broken. Should I expect v1.8.0 will introduce something else I can't turn off without a lot of less-than-obvious hacking?

What angers me most is the total lack of regard for users who think differently. Instead of allowing them an option to turn off unexpected (and pretty invasive) new workflow changes, you expect them to fool around in their init scripts or keymaps. This demonstrates total disregard for your userbase.

I mean, sure, the keymaps and hacky false-pending-pane-mode workarounds work. For now. But I can't shake the feeling they're temporary solutions until something else changes for no logical reason.

@AEgan
Copy link

AEgan commented Apr 19, 2016

  1. Easily understandable for new Atom users (because it more closely matches the behavior of other editors)

I'm not saying that the team doesn't take this into consideration already, but in this scenario the change broke the workflow of current Atom users, and the first listed reason was so that new users wouldn't need to learn a new workflow. People who switch to a different editor do so knowing their workflow will change. People who upgrade to a new minor release may not have the same expectation.

I appreciate the difficult task it must be to maintain Atom in a way that allows users to so easily voice their frustrations around decisions you make, but please be careful when removing features from users in a minor release.

@einkoro
Copy link

einkoro commented Apr 19, 2016

While I don't expect this to change its a jarring UI/UX change pushed out as default and not giving an option to revert back to the previous behaviour seems completely asinine to me. It is not the behaviour I would expect from any file tree regardless of operating system – the only other time I've encountered it was in sublime and it was an immediate deal breaker due to how unnatural the entire process was not to mention RSI inducing extra clicks for opening a file which is about the last thing anyone's wrists and arms need on a regular basis in a industry where we sit at computers for eight hours or more per day.

@ebordeau
Copy link

It's ridiculous that you took away functionality and broke current users' workflow simply to make new users more comfortable... especially in a minor version update! Thankfully, there's a package that fixes this problem (and it is a problem). Having single click do nothing?! I have concerns about the future of Atom when decisions like this are made.

@bjmiller
Copy link

For the many multitudes of people who disagree with this change, I would like to urge you to log in and add a star to @olmokramer 's single-click-open package. At the very least, it should be in the tending list for packages. When more people use it than don't, maybe that will help drive home the point.

@pengbits
Copy link

+1 @olmokramer you are a lifesaver

@newscloud
Copy link

This is a terrible compromise decision. Add an additional configuration option.

I was unable to get @olmokramer fix to work... I tried both his one line fix with Allow Pending Pane Items on and the multiline fix with the setting off. I added these separately to init script. Is that correct?

@Ben3eeE
Copy link
Contributor

Ben3eeE commented Nov 24, 2016

@newscloud the fix by @olmokramer was added as a package single-click-open. I suggest you try that out and if it doesn't work report an issue on that repository, thanks!

@rc2524
Copy link

rc2524 commented Aug 3, 2017

Sorry to bring up an old thread. I'm a new Atom user and this was a super confusing feature. So I disagree with @lee-dohm's first point.
I also disagree with his second point since I don't have the option to do what I want in the settings. At least there's a plugin to fix it (thanks @olmokramer!).

@damianprzygodzki
Copy link

Back from a journey on backend and i see it is a still issue :)

The solution is easy for me now. It's not a editor for me. Wrong choices should have reflection in statistics.

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 9, 2018

It's not a editor for me. Wrong choices should have reflection in statistics.

... uhhh, YEAH! Shame on you, Atom team, for replacing my beloved text-editor with rusty x-ray equipment, a game of real-time code tetris, and something about a tree-house. Atom used to be THE hackable editor for the 21ST CENTURY, but you ditched us all to write next-generation code for the 32nd Century instead. You no longer support your current-generation, or even the previous-generation, or the one before that. You all make me SICK!!

ALL BULLSHIT ASIDE …

Look at how far Atom has progressed in the last two years, @damianprzygodzki. Seriously.

Wrong choices should have reflection in statistics.

It was a wrong choice. The outcome of this PR makes no more sense to me now than it did then.
But the Atom guys took the flak and wore the heat of user complaints, and they made up for one retarded decision by making an immeasurable number of right decisions. They worked their arses off to make enormous improvements to the Atom's usability and performance, all whilst keeping considerable grace under fire with breaking changes during migration periods.

You can't possibly compare the Atom we have today with the bloated sack of CoffeeScript it was in April 2016, and say "this still sucks, I still have to right-click". Do what I do every time there's some annoying new shit I hate and hack the crap out of it until it's right again! Hell, it's one line of JavaScript in your init.js file even:

atom.workspace.onDidAddPaneItem(e => e.pane.setPendingItem(null));

If you think that's hacky, I got news for you son: you ain't seen nothin' yet. Although what I was reduced to recently was going too fucking far, and this paragraph convinces me I should really send them a pull-request. Hmmm...

Anyway. Bottom-line is the Atom team kept putting their backs into this editor, and they're still bringing new stuff to the table. Admittedly, most of it is stuff I'll never use: realtime collaboration is meaningless to me because I'm an intolerant control freak who hates your code style, but that's beside the point. 😅

@Alhadis
Copy link
Contributor Author

Alhadis commented Apr 9, 2018

inb4 that sodding @lock bot spams me with another automated notification to tell me another ancient thread from two years ago just got locked to prevent users from adding meaningless drivel.

@atom atom locked as off-topic and limited conversation to collaborators Apr 9, 2018
@lee-dohm
Copy link
Contributor

lee-dohm commented Apr 9, 2018

Thanks everyone for the feedback over the years on this. We know that many people don't like this feature or this decision. As @Alhadis points out though, Atom provides many more customization opportunities than simply via settings. We want people to have the ability to craft an editor that works the way they want it to. Unfortunately, every line of code and every setting that is added to Atom Core or the built-in packages must be maintained in perpetuity by a small group of maintainers. So sometimes we have to make tough choices and we can't please everyone. If that means that some people end up using other editors, that's fine too. We just want you all to be happy 😀

I've locked the conversation here because it had ceased being a constructive discussion. If people have constructive feedback they want to share with the Atom team over this, you may contact me at atom@github.com.

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

Successfully merging this pull request may close these issues.