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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add open in new tab link #3059

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

obourgain
Copy link

@obourgain obourgain commented Mar 20, 2024

Description
For each file in the file list, add a link to open it in a new tab.
This is useful when you have a folder with a lot of files, as otherwise when hitting the back button, you may have to scroll the file list again.

My current code doesn't display a pretty button or anything, but it works for me. Would you open to integrate a feature like that?

馃毃 Before submitting your PR, please indicate which issues are either fixed or closed by this PR. See GitHub Help: Closing issues using keywords.

  • DO make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your master!
  • DO make sure you are making a pull request against the master branch (left side). Also you should start your branch off our master.
  • DO make sure that File Browser can be successfully built. See builds and development.
  • AVOID breaking the continuous integration build. / I ran make test frontend

Further comments

Screenshot of filebrowser serving itself with this PR:
image

@obourgain obourgain requested a review from o1egl as a code owner March 20, 2024 20:04
@niubility000
Copy link
Contributor

niubility000 commented Mar 21, 2024

It's already fixed in #3004 and #3052 in a common way. you can test it here:https://github.com/niubility000/filebrowser/archive/refs/heads/feat-005-stay-in-the-same-position-of-the-list-when-back-from-a-sub-folder.zip
Your solution is so simple and smart. It will open a new tab everytime when you click the item's "open in new tab" in the list. But the UI with"open in new tab" in every item looks a little bit weird, and it doesn't work in the simple list mode, and in iphone the behaviour is a little bit weird.

@obourgain
Copy link
Author

I do agree that the layout is not good, especially in list mode.
Also, it could be an icon instead of text, e.g. in this style https://thenounproject.com/browse/icons/term/new-tab/ (I didn't check the licence for those, it's just for example).

I don't understand "it doesn't work in the simple list mode". It doesn't look pretty, but the link is there and opens in a new tab. The layout issue is fixable, and an icon could be prettier, but I first wanted to first know if you are interested in this feature or not.
image

What is weird with iPhone? I tried on my android, and it opens a new tab.

Another problem is the "Use single clicks to open files and directories". If this is enabled, clicking the "open in new tab" also triggers the "onClick" of the item.

@tox2ik
Copy link

tox2ik commented Mar 31, 2024

This is a terrible way to add this standard behavior (bear with me, i'm not criticizing you):

  • It clutters the UI and looks bad.
  • Where else have you seen a link like this - ask yourself?
  • You can open in a new tab by holding ctrl and clicking a link in many other web-apps.
  • You can open in a new tab with "right+click -> open in new tab" (right click = long press on android)

OR...
if you can't ctrl+click it's because the name of the folder is not an anchor, as it should be. So better fix that.

Right now there is a div[role=button] type of element that serves as the clickable part. This is "bad" (but useful because the whole area is clickable);

To make this semantically and functionally correct there should be an anchor around the name of the folder <a href=/files/folder-33>folder33</a> instead of the current <p class=name>{{name}}<a/>

At the very least, what is proposed in this PR, I think should be an opt-in, default-off feature via user-profile settings, but hopefully you agree that the <a>{{name}}</a> solution is the better way to solve this. I mean without introducing a new button that clutters the UI and without doing something that no other browser known to man has ever done.

So overall, I do see the utility in being able to open in a new tab, but the current solution is not good. I have proposed an alternative approach that is the standard way to solve this in HTML, and would be supported on mobile via "long press" or what have you...

@o1egl
Copy link
Member

o1egl commented Apr 24, 2024

Thank you for this proposal, but from the ui/ux perspective this implementation is not good. I agree with @tox2ik . I think it would be beter if we simply use the native right-click popup window
Screenshot 2024-04-25 at 01 34 07

@kloon15
Copy link
Contributor

kloon15 commented May 5, 2024

U can just simply use middle click to open a link in new tab.

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

Successfully merging this pull request may close these issues.

None yet

6 participants