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

An option to navigate inside the tree at sidebar the 'traditional' way when using arrow keys? #323

Open
nik-gr opened this issue Jan 19, 2023 · 13 comments
Labels
enhancement New feature or request

Comments

@nik-gr
Copy link

nik-gr commented Jan 19, 2023

As it is now when moving around using arrows, on each movement the selected item automatically goes at top and left (scrolling accordingly on both directions). I guess like that can make it easier to start a capture with the dragging method but personally I never succeded to become familiar with this automatitation and I am using mouse to move around the tree. Can you pls provide an option so when moving with arrows to be in the normal way (only scrolling up/down and only when the selected item is at the top/bottom)

(+) Even better if it's added a way to also make it possible to scroll, for example with alt+arrows

(+)If it is easy to implement also a behavior where ctrl+click on an item that will open it in a new tab (instead of the same tab when it is a simple click - similar to ctrl+click of links in webpages)

@danny0838
Copy link
Owner

As it is now when moving around using arrows, on each movement the selected item automatically goes at top and left (scrolling accordingly on both directions). I guess like that can make it easier to start a capture with the dragging method but personally I never succeded to become familiar with this automatitation and I am using mouse to move around the tree. Can you pls provide an option so when moving with arrows to be in the normal way (only scrolling up/down and only when the selected item is at the top/bottom)

We'd like to. However the behavior is controlled by the related browser API, and cannot be easily changed. A pure script re-implementation of the full behavior by a UI expert is likely required.

(+) Even better if it's added a way to also make it possible to scroll, for example with alt+arrows

This may be possible but need further evaluation.

(+)If it is easy to implement also a behavior where ctrl+click on an item that will open it in a new tab (instead of the same tab when it is a simple click - similar to ctrl+click of links in webpages)

Ctrl-click is for selection control. We won't implement this due to conflict.

It's more recommended to open in a new tab through middle click, which is also implemented by most modern browsers.

@danny0838 danny0838 added the enhancement New feature or request label Jan 20, 2023
@nik-gr
Copy link
Author

nik-gr commented Jan 22, 2023

I thought it was on purpose to have the focused item on top.

  1. removing scrollIntoView commands inside booktree.js - keyboardNavigation ecxept the parent.scrollIntoView() one
  2. at tree.js, inside keyboardNavigation at ArrowUP replacing target.scrollIntoView(); with
if(target.getBoundingClientRect().top <= document.documentElement.querySelector('li:not([data-type="folder"])')
.offsetHeight){    target.scrollIntoView();    } 

  1. at tree.js, inside keyboardNavigation at ArrowDown replacing target.scrollIntoView(); with
if(target.getBoundingClientRect().top >= document.documentElement.getBoundingClientRect().height-document.documentElement.querySelector('li:not([data-type="folder"])')
.offsetHeight){     target.querySelector("a").scrollIntoView(false);            }

It has some issues:
-under some circumstances the tree will be scrolled right, not showing initial part of item's names (until the next scroll happens)
-I commented out the part where with right arrow you go to the 1st child of a folder, if that remains needs to have some appropriate check probably the same as for Arrowdown
-If there is a scrolling up with mouse so that focused item is not displayed, any following Arrowdown (or scroll down+Arrowup) will leave the focused items outside of the viewable area.
can be fixed with adding the check for Arrowup to Arrowdown check and vice versa although for avoiding the unnecessary extra checks I would prefer a key shortcut for regaining focus by calling scrollIntoView (besides, it also autocorrects just by pressing the opposite arrow key)
? I'm not sure if there are any conditions where pressing space could lead to scrolling, if it does then there shall be some check there as well

@danny0838
Copy link
Owner

I think the root issue is that the scrollIntoView API scrolls both horizontally and vertically even if unneeded. To fix the issue we have to either find a way to prevent it with a redesigned HTML/CSS, or get rid of it and emulate most of its behavior.

@nik-gr
Copy link
Author

nik-gr commented Jan 22, 2023

With the modifications above, vertical scrolling happens as expected and only when needed. Horizontal scrolling can be a little off under specific conditions (if the user folds a folder and move to items in parental folders - until it reach the top/bottom of the window). Actually even this case will be eliminated with that

with adding the check for Arrowup to Arrowdown check and vice versa

(I'll check it tonight)

@danny0838
Copy link
Owner

We will get the same issue as long as scrollIntoView is called.

We have implemented an adjustment of its parameters to get it work in a more desirable way (it will scroll left back when going to an upper item). You can test it from the devel branch.

@nik-gr
Copy link
Author

nik-gr commented Jan 22, 2023

Yes you are right it wouldn't have worked. I didn't give it much of a thought.
I tried the new version. The way it scrolls left-right it's not ideal, I'm not sure in the long term how annoying will become or if it's something you get used to it.
But it still has the same bug with the initial one, when a folder element opens, its size changes so what it happens with the new version is that while pressing down arrow, out of nowhere it will move the focused item to the top.
That is why in the code I wrote above I put its <a> child (which keeps the dimensions of the simple tree items) to call the scrollIntoView

The only reason I kept scrollIntoView is because when I tried to test it yesterday I couldn't find how to scroll the sidebar. If that's trivial then the solution is easy, the only times that there is a need for scrolling after Arrow up/down/left/right, is at the 3 places I mentioned in the post above and for a scroll = tree item height. Plus the case when there is a scroll using mouse that will hide the focused item which can be solved with a shortcut that will call scrollintoview to bring it at the top of the tree
(and horizontal scrolling be reset to 0 after every scrollintoView call in the code)

@danny0838
Copy link
Owner

danny0838 commented Jan 23, 2023

I think automatic horizontal scrolling is reasonable as there is a horizontal scrollbar, which is different from most "traditional" file manager like implementation, which either supports only single level navigation or don't support horizontal scrolling at all (and deeper items will be clipped forever).

If some UI expert can contribute a neat way to emulate scrollIntoView, we may consider adding an option to force vertical-only scrolling, or possibly horizontal scrolling only when horizontally invisible.

This change aims to prevent the issue that the horizontal scrolling goes right when entering a deeper level but doesn't go left back when going back to a shallower level, which seriously interferes keyboard only navigation. We'll go for this (at least temporarily) unless it's proven worse or there's a better practical approach.

@nik-gr
Copy link
Author

nik-gr commented Jan 24, 2023

Horizontal scrolling of course is important, the width of sidebar is small in most cases you cannot see the full title, if needed you must be able to scroll for that.
This auto-scrolling though it's so small that don't really offers much, I mean it's roughly more than a char at each level, going 4 folders deep will have a viewable with just a small size word extra. And the constant left-right movement of the entire tree at least as 1st experience seems not pleasant (with frequency depending on the tree structure and the usage of the navigation)

On the other hand if you provide shortcuts for scrolling as you are considering, with the approach I proposed it will remain with any scrolling the user gives until he changes it or proceed closing a folder in which case it will go to the left most position (or if undesirable avoid that as well with 2 lines of extra code to restore the scroll position as it was before the closure of the folder)

@danny0838
Copy link
Owner

As you may have found out, there is no simple reliable way to find an appropriate ancestor element for scroling. That is not a practical approach until some expert have contributed one.

@nik-gr
Copy link
Author

nik-gr commented Jan 25, 2023

scroll.zip
Check this. It handles the approach as I explain above and solves the bug I mentioned (random scrolling to the top).
With an addition of scrolling shortctuts it's a complete solid solution, which actually is quite trivial but I didn't want to mess your code (the only alt combination I can see is with mouse click that provides nothing essential that is not already provided from ctrl+mouse click)

-If mouse scrolling moves focused item out of view it's already possible to handle by pressing space which will bring it back to top
-The implementation moves to left most position whenever a folder closes which I find desirable, the only use case of horizontally scrolling I can thing of is to see the full title of an item. If you prefer to have any manual scrolling remain until manually changed I have it in the comments.
-For simplicity of code I removed the movement to 1st child with right arrow (I don't get its importance, it is a movement happening with down arrow anyway). If you choose to keep it, needs the same code as for down arrow.

@danny0838
Copy link
Owner

Your approach is not valid. There is no guarantee that the tree structure must be put under a scrollable element whose ID is "tree". A large code rework will be involved if we really want to change it, which is not something we'll consider shortly.

@nik-gr
Copy link
Author

nik-gr commented Jan 26, 2023

I don't follow your reasoning. Did you check the code? It is valid

@nik-gr
Copy link
Author

nik-gr commented Jan 26, 2023

keyboard navigation needs booktree.js to be loaded, all cases when that is happening are covered
(but yes having the tree scrolling element take the same id/special attribute/class regardless where and how is implemented (in present and any future cases) would be better)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants