Skip to content

fix(FilesPicker): Prevent selection of nodes without create permission - #55796

Merged
artonge merged 5 commits into
masterfrom
artonge/fix/hide_non_writable_nodes
Oct 22, 2025
Merged

fix(FilesPicker): Prevent selection of nodes without create permission#55796
artonge merged 5 commits into
masterfrom
artonge/fix/hide_non_writable_nodes

Conversation

@artonge

@artongeartonge commented Oct 15, 2025

Copy link
Copy Markdown
Collaborator

It is misleading to show those nodes in the file picker, as picking them will always result in an error.

Todo:

The new version will also prevent navigation to non-readable folders. This will prevent navigating to E2EE folders.

BeforeAfter
imageScreenshot From 2025-10-17 11-17-19

@artonge
artonge requested review from a team and skjnldsv as code ownersOctober 15, 2025 14:50
@artonge
artonge requested review from sorbaugh and susnux and removed request for a teamOctober 15, 2025 14:50
@artongeartonge self-assigned this Oct 15, 2025
@artonge
artonge removed the request for review from sorbaughOctober 15, 2025 14:53
@artongeartonge added this to the Nextcloud 33 milestone Oct 15, 2025
@susnux

Copy link
Copy Markdown
Contributor

I think we discussed this somewhere with Jan, the outcome was that it is even more confusing to not see all your files.
The solution that was agreed is to grey out non-selectable files.

We probably need a second filter way for this on the library though...

@artonge

Copy link
Copy Markdown
CollaboratorAuthor

We probably need a second filter way for this on the library though...

Or we instead gray out the files based on that already existing function? Is there a specific use case where we actually want to filter out some nodes?

Comment threadapps/files/src/actions/moveOrCopyAction.ts Outdated
@artonge

Copy link
Copy Markdown
CollaboratorAuthor

I don't think this will work.

We might not have create permission on a specific folder, but still have them on one of its subfolders. Groupfolders with ACLs for example.

So we still need to be able to navigate into those folders.
But for E2EE folders, it makes sense to not be able to navigate to them.

Maybe we also need a canNavigate function?

@susnux

Copy link
Copy Markdown
Contributor

But for E2EE folders, it makes sense to not be able to navigate to them.

Then you should be able to navigate into those greyed folder but not be able to pick them.
Thats all that this prop says: not pickable

@artonge

artonge commented Oct 17, 2025

Copy link
Copy Markdown
CollaboratorAuthor

Then you should be able to navigate into those greyed folder but not be able to pick them.

No, for E2EE it does not make sense as we cannot even load their content. But luckily, they are also not readable, I'll push a better approach in a few minutes.

Edit: done

@artonge
artongeforce-pushed the artonge/fix/hide_non_writable_nodes branch from 37e2ebd to f2ab1c3CompareOctober 17, 2025 09:20
@artongeartonge changed the title fix(FilesPicker): Filter out non writable nodesfix(FilesPicker): Prevent selection of nodes without create permissionOct 17, 2025
@artonge
artongeforce-pushed the artonge/fix/hide_non_writable_nodes branch from f2ab1c3 to 9216214CompareOctober 17, 2025 10:36
@artonge
artonge requested a review from susnuxOctober 17, 2025 10:40
@artonge
artongeforce-pushed the artonge/fix/hide_non_writable_nodes branch 2 times, most recently from 3ffa65c to 35ec297CompareOctober 17, 2025 13:08

@jancborchardtjancborchardt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

Might additionally be good to have some textual support there, like a title on hover & focus which says "No permissions to upload to this folder", for example.

@artonge
artongeforce-pushed the artonge/fix/hide_non_writable_nodes branch from 35ec297 to 2f97714CompareOctober 21, 2025 15:14
@artonge
artonge requested a review from a team as a code ownerOctober 21, 2025 15:14
@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/compile /

@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/compile rebase /

artongeand others added 5 commits October 21, 2025 20:27
And remove the disable callback for the button as it is not needed anymore.
This gives a better experience as the nodes do not have a pointer for cursor, and the submit button does not react when clicking on them.
Signed-off-by: Louis Chmn <louis@chmn.me>
Signed-off-by: Louis Chmn <louis@chmn.me>
Signed-off-by: Louis Chmn <louis@chmn.me>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nextcloud-command
nextcloud-commandforce-pushed the artonge/fix/hide_non_writable_nodes branch from f45ef38 to 58e9be7CompareOctober 21, 2025 20:30
@artonge
artonge merged commit f2eb772 into masterOct 22, 2025
124 checks passed
@artonge
artonge deleted the artonge/fix/hide_non_writable_nodes branch October 22, 2025 05:18
@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/backport cb062d2 to stable32

@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/backport cb062d2 to stable31

@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/backport cb062d2 to stable30

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Remove files from the list of recent items in the move/copy modal

4 participants

@artonge@susnux@jancborchardt@nextcloud-command