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

Add config to determine if tree selection changes when editor changes #1335

Open
bobjunga opened this issue Mar 14, 2020 · 2 comments · May be fixed by #1336
Open

Add config to determine if tree selection changes when editor changes #1335

bobjunga opened this issue Mar 14, 2020 · 2 comments · May be fixed by #1336

Comments

@bobjunga
Copy link

bobjunga commented Mar 14, 2020

Summary

Change the config around autoReveal so that its possible to turn off the automatic tracking of the active editor Pane Item.

I have already coded Option1 (see below) but not sure if its the best way so I am making the feature request to discuss before submitting the PR.

Option1 Working Code:
repo: [email protected]:junga-com/tree-view.git
branch: feature-select-active-file-config-option

Motivation

Sometimes I do not want the tree selection to change as I cycle through my open tabs or close tabs.

Scenario: I want to explore one section of my large file hierarchy -- open a file .. read it ... close it ... go back to tree and move the cursor up and down to open a neighboring file (all with keyboard).

What happens today is as soon as I close the file, the selection changes to what ever new tab is and I am no longer exploring the same location in the hierarchy.

With the current autoReveal config off, at least the tree view does not immediately change, but when I focus the tree-view with a keystroke and then try to navigate with keystrokes, the tree scrolls unpredictably away from where I am looking. The is particularely bad if the active item is the SettingsView because the selection is cleared from the tree and my next up of down arrow goes all the way to the bottom of my hierarchy.

When I was new to Atom, the autoReveal setting appeared to be just what I was looking for but it was very confusing when it still automatically revealed which editor was active but sometimes I could not see the selection (so it would appear to deselect the tree selection).

Describe alternatives you've considered

Option 1:
Change Auto Reveal to a 3 option enum instead of a boolean. Two states would correspond to the existing behavior which is to automatically track the active WorkspaceCenter Item, with or without scrolling when needed. The third option would disable the tracking behavior.

I actually coded this option but I have doubts now and think that Option 2 might be better..

Advantages to Option 1 are that 1) The name is familar and works better than before because now you can make it not-auto, auto-withoutScroll or auto-withScroll and 2) I was able to make it backward compatible with existing configs to preserve the user's choice.

Option 2:

  1. Add a new boolean config to control whether the tree selection changes when the active editor changes.
  2. The existing autoReveal would only apply if the new config is 'true'

Option 2 has the advantage that when you toggle the new option on and off, it remembers if you what the auto-selection mode to scroll or not.

I think that the reason I did not do Option 2 first is that the naming of "autoReveal" sounds like its controlling whether it will do something automatically when the active editor changes. I think that with this option, we should consider renaming the existing autoReveal config and I am not sure ho to preserve the exiting configs in that case.

I think that ideally the configs could be:

autoReveal -- boolean - (new option to control if tree selection changes at all when editor changes).
autoRevealScroll -- boolean -- (controls whether to scroll when needed if autoReveal is true)

.. but the new setting can't have the same name as the existing setting because of existing configs.

So maybe...
autoTracActivePane -- boolean - (controls if tree selection changes at all when editor changes).
autoReveal -- boolean -- (existing setting as is -- but its a bt of a misnomer)

Additional context

Screenshot of Option 1 Settings View...

@bobjunga
Copy link
Author

I made a new branch with Option 2 which is better in every way now than Option 1. I think I will make a PR for it unless someone chimes in soon.

  • the defaults are Enabled:true, AutoScroll:false which maintains the same default behavior as before this change.
  • it migrates the old autoReveal if its present in the config.
    • This is required not only to keep the user's preference, but also we need to call unset on it so that the Settings View does not keep displaying it even though its no longer in the schema.
  • note that I also changed the description of the "Focus On Reveal" config to make it clear that it does not affect Auto Reveal but just the Reveal Active File command.
  • I believe that this is 100% non-breaking, backward compatible.

Option 2 Config Settings screenshot

bobjunga added a commit to junga-com/tree-view that referenced this issue Mar 17, 2020
* closes atom#1335
* the defaults are Enabled:true, AutoScroll:false which maintains the same default behavior as before this change.
* it migrates the old autoReveal if its present in the config.
  * migration is required not only to keep the user's preference, but also we need to call unset on it so that the Settings View does not keep displaying it even though its no longer in the schema.
* note that I also changed the description of the "Focus On Reveal" config to make it clear that it does not affect Auto Reveal but just the Reveal Active File command.
* I believe that this is 100% non-breaking, backward compatible.
@bobjunga bobjunga linked a pull request Mar 17, 2020 that will close this issue
@bobjunga
Copy link
Author

I created a pull request for option 2. Here is a picture of the Settings UI thats in the PR. I changed the wording after I realized that 'reveal' means that the tree will expand the parent nodes as well as scroll.

AutoRevealConfigChange

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

Successfully merging a pull request may close this issue.

1 participant