Filelist: Intended behavior that scrollTo opens detailsView?

[te-online]

Just a quick question if this is intended behavior, it seems a bit odd to me:

Steps to reproduce

1. Create a new file with the files_linkeditor app (for clarity: it uses the filelists core addMenuEntry method, hence the issue in this repository). (see Display glitch with sidebar/details view, when creating a new link file te-online/files_linkeditor#18)
2. The scrollTo method is invoked on the newly created file.
3. The detailsView pane is opened and focused.

Expected behaviour

For new files, the scrollTo method is invoked, the file list scrolls, the new file is highlighted, but the detailsView pane stays hidden until the user clicks Details.

Actual behaviour

The detailsView pane is opened and steals focus. (see https://github.com/nextcloud/server/blob/master/apps/files/js/filelist.js#L2969)

Rest...

irrelevant for this issue

[te-online]

I created a little hotfix which might not be the preferred solution, but could work anyways.
#12990

[juliusknorr]

Thanks for your pull request @te-online. I think we could just get rid of opening the details view when creating a new file. For new files they are empty, so the most common usecase would be to open the file for editing, right? cc @nextcloud/designers

[jancborchardt]

For new files they are empty, so the most common usecase would be to open the file for editing, right?

Yes. This already happens with files_texteditor. If there is no viewer or anything, opening the sidebar is proper.

Same with creating new folders: It should automatically open the folder. I thought there was an issue for that also but I can’t find it anymore?

[te-online]

I have to look into how files_texteditor does it to replicate in files_linkeditor. Do you have any hints on where to find that code @jancborchardt? :-)

I agree with you, @juliushaertl, in not seeing a point in opening the details panel for new files. I personally never used it, but haven't observed any users complaining about it, either (aside from the problem in my own app).

[juliusknorr]

Same with creating new folders: It should automatically open the folder. I thought there was an issue for that also but I can’t find it anymore?

Not sure about that, since after creating a folder another popular use case might be to move files from the current directory into that newly created folder. Also this is no behaviour I know from any existing file manager.

[juliusknorr]

@te-online This is the code section you might be looking for: https://github.com/nextcloud/files_texteditor/blob/master/js/editor.js#L700-L703

[te-online]

Okay, I'm a little confused as to how we're going to continue? 😉 I'm going to have at the editor.js, thanks, @juliushaertl. And apart from that I guess I'll close the issue if I can resolve it for files_linkeditor in the same way as files_texteditor does it, right?

[jancborchardt]

And apart from that I guess I'll close the issue if I can resolve it for files_linkeditor in the same way as files_texteditor does it, right?

Yep – we have to solve it on a case-by-case basis. The default is open the viewer, and if there is no viewer we open the sidebar at least.

For folders it will stay like now as @juliushaertl correctly mentioned that opening directly "is no behaviour I know from any existing file manager."

[te-online]

Hi all and thanks for your help, @jancborchardt and @juliushaertl.
Since I have investigated a little further, I'd like to go over this one more time:

Problem description

The issue consists of two problems:

1. The sidebar opens inevitably when creating a new file and is z-indexed on top of .oc-dialog (z-index: 1500 vs z-index: 1000)
2. The comment field in the sidebar focuses, as soon as the sidebar opens. Since it opens asynchronously (because of pending ajax requests), a race condition exists, if your file editor also focuses asynchronously. This also concerns files_texteditor. If the initialization of ACE would be faster, I guess, the sidebar would also steal focus here.

Alleged solution in files_texteditor

I found that files_texteditor uses the exact same code as files_linkeditor. The only difference is that the texteditor opens in a modal that overlays the opening sidebar (higher z-index), effectively “hiding” the first problem and the duration of initialization does the job of focussing the editor after the sidebar has been focussed, avoiding the 2nd problem.

Both of them don't really happen on purpose. Both issues really make it a lot harder for apps registering a addMenuEntry action handler for creating new files.

Possible solution

I'd still argue for adding a flag to the addMenuEntry function, as proposed here: #12990 – I believe that this would be the “cleanest” solution. If there are reasons for not doing that, I'd appreciate to hear them an then re-evaluate my opinion 😉

As a quick fix for files_linkeditor, I'll bump up the z-index on my overlay and add a dirty setTimeout to my focus call.

PS: I hope this post didn't sound rude... 😳

[matiasdelellis]

Please, this is a bit annoying.
If I search for a file from the top panel, it seems good that I open the file, but the behavior of displaying the selected file should work correctly.
Once I found a file, I should be able to move around it to see other files and it's impossible! 😞

[kesselb]

cc @juliushaertl @jancborchardt

[ghost]

This issue has been automatically marked as stale because it has not had recent activity and it seems to be missing some essential informations. It will be closed if no further activity occurs. Thank you for your contributions.

[te-online]

I'm wondering, @jancborchardt, what information is missing in context of this issue? 🤷‍♂ Did you read my detailed explanation of the issue? Happy to provide more info or test again!

[kesselb]

cc @juliushaertl @jancborchardt

"needs info" because we are waiting for a response. stale bot added the "stale" label, you received a notification and mentioned on of the developers again. I think the workflow worked quite well 😀

[juliusknorr]

Yep – we have to solve it on a case-by-case basis. The default is open the viewer, and if there is no viewer we open the sidebar at least.

In that case the additional flag proposed by @te-online sounds like the best way to do that:

I'd still argue for adding a flag to the addMenuEntry function, as proposed here: #12990 – I believe that this would be the “cleanest” solution. If there are reasons for not doing that, I'd appreciate to hear them an then re-evaluate my opinion wink

We just need to make sure the default is open and we set the flag to just scroll to the file for folders then.

[te-online]

I could try reproducing this PR onto the current codebase #12990
Would that work?

[juliusknorr]

Yep, sounds good to me.

[te-online]

Quick update: It will take me a while more to get to this, because I need to check the new position of this code or if it changed. So it's not a 5-minute thing and thus far down the todo-list. But I'll post an update as soon as I get to it 🙂

[te-online]

The PR should still work, that method createFile did not change. So I reopened it: #12990 :-)

[szaimen]

I think this is superseded by #27100