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

File openings inside new tab #2521

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

File openings inside new tab #2521

wants to merge 11 commits into from

Conversation

trollepierre
Copy link
Contributor

@trollepierre trollepierre commented Feb 23, 2022

This PR handles most of the case I found:

  • Drive view
  • Recent view
  • Sharing view/index (click on "/" path below a folder can have unexpected behaviour)
  • Sharing view/ inside a folder
  • Trash (not verified a last time)

Handle:

  • img/mp3/txt file (so most common file)
  • notes
  • shortcut
  • folder

But has not been cleant / tested at all.

⚠️ Won't fixed => to add in another story:

  • in RecentView, right click on folder filepath MidEllipsis (because this span is located inside FileName inside FileOpener, that is an anchor opening the recent file - not the folder)

comment about this gif:

  • the link below the anchor is wrong (it's the same than the file of the table cell) => ❌ you can see on the below left corner on my browser

  • when Cmd + Click, we have the expected feature (folder opened in a new tab) => ✅ I arrived inside the test folder
    2022-03-10 10 05 37

  • when right click, opening, we open the file instead of the folder => ❌ I arrived on the Recent gif, instead of the folder test

  • on click on Notes / Shortcut, this needs the big refactor "Don't use on click, use only anchor - prefetch (query?)"

onFolderOpen(attributes.id)
console.log('is directory')

if (event.ctrlKey || event.metaKey || event.shiftKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems that you build an href for a folder https://github.com/cozy/cozy-drive/pull/2521/files#diff-66f571a15fdeccd6293e111e9483d731a1d8a1bddf527123ece478a9605e12bfR121 why do you need to check the event key?

let buildHref = ''
if (isFolder(file)) {
buildHref = `/#${folderUrlToNavigate(file.id)}`
} else if (isNote(file)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Crash--
Copy link
Contributor

Crash-- commented Feb 24, 2022

The thing I dislike here is that we still have several ways to handle navigation. Sometimes we will have an href, so we'll be able to make a right click and use browser action, sometimes not since the href will be empty. Right?

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.

2 participants