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.

Sign up for GitHub

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

Jump to bottom

feat(sidebar): Show node owner in metadata subline #46691

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

feat(sidebar): Show node owner in metadata subline #46691

wants to merge 1 commit into from
+72 −25

Conversation

Fenn-CS
Copy link
Contributor

@Fenn-CS Fenn-CS commented Jul 22, 2024 •
edited
Loading

Summary

Show user bubble to indicate owner of file in sidebar overview (this should not change irrespective of share status)...

Resolves: #46178

Screenshots

Timeline State
Before metadata-before
Now (owned by you) metadata-owned-by-you
Now (owned by other) metadata-owned-by-other

TODO

  • Finish wip
  • Screenshots

@Fenn-CS
Copy link
Contributor Author

Fenn-CS commented Jul 25, 2024

Show original file owner in side bar

Now that some logic to show the owner is in place with a simple "draft" to show this information with the userbubble near other folder metada can @nextcloud/designers chime in?

show-admin

@Fenn-CS Fenn-CS marked this pull request as ready for review July 25, 2024 02:55
@Fenn-CS Fenn-CS requested a review from skjnldsv as a code owner July 25, 2024 02:55
@Fenn-CS Fenn-CS requested review from susnux and artonge July 25, 2024 02:55
@nimishavijay
Copy link
Member

Nice! Good idea using the user bubble component. I would suggest we include only the username in the user bubble, and to separate the different metadata we could use a middle dot ·. So it would look like

11.2 MB · 3 weeks ago · Owned by (👱🏼 Alex Doe)

Also the user bubble component should be center aligned with the rest of the text :)

@skjnldsv skjnldsv marked this pull request as draft July 25, 2024 09:20
@Fenn-CS Fenn-CS marked this pull request as ready for review July 26, 2024 02:30
@Fenn-CS Fenn-CS force-pushed the fix-show-original-owner branch 2 times, most recently from 9c1c432 to 1a55a34 Compare July 27, 2024 01:37
@Fenn-CS Fenn-CS changed the title Wip : Show owner information in sidebar feat(sidebar): Show node owner in metadata subline Jul 27, 2024
@skjnldsv skjnldsv removed the 3. to review Waiting for reviews label Jul 27, 2024
nickvergessen
nickvergessen reviewed Aug 14, 2024
View reviewed changes
apps/files/src/views/Sidebar.vue Outdated Show resolved Hide resolved
apps/files/src/views/Sidebar.vue Outdated Show resolved Hide resolved
@sorbaugh sorbaugh added this to the Nextcloud 31 milestone Aug 14, 2024
@sorbaugh sorbaugh requested a review from susnux September 5, 2024 14:06
@Fenn-CS Fenn-CS force-pushed the fix-show-original-owner branch 2 times, most recently from 50600f2 to c10edef Compare September 17, 2024 14:45
artonge
artonge reviewed Sep 18, 2024
View reviewed changes
apps/files/src/views/Sidebar.vue Outdated Show resolved Hide resolved
apps/files/src/views/Sidebar.vue Outdated Show resolved Hide resolved
artonge
artonge reviewed Sep 19, 2024
View reviewed changes
apps/files/src/views/Sidebar.vue Outdated
@@ -460,6 +486,7 @@ export default {
this.fileInfo = await FileInfo(this.davPath)
// adding this as fallback because other apps expect it
this.fileInfo.dir = this.file.split('/').slice(0, -1).join('/')
this.node = await fetchNode({ path: (this.fileInfo.path + '/' + this.fileInfo.name).replace('//', '/') })
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't we fetching the file info already somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I doubt in the same way we need it here, @skjnldsv is there somewhere else I can pick this up?

Copy link
Contributor

@susnux susnux Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

As the sidebar is part of the files app, you can use the files store to retrieve the current node without needing to re-fetch it again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the fileStore as at now breaks the sidebar. With undefined errors.... Looks like the sidebar is trying to reference the fileStore before it is ready.

apps/files/src/views/Sidebar.vue Show resolved Hide resolved
@Fenn-CS Fenn-CS force-pushed the fix-show-original-owner branch 2 times, most recently from cb22b2d to 908165a Compare September 19, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sorbaugh sorbaugh sorbaugh approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv skjnldsv is a code owner

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

@susnux susnux Awaiting requested review from susnux

@artonge artonge Awaiting requested review from artonge

At least 2 approving reviews are required to merge this pull request.

Assignees

@Fenn-CS Fenn-CS

Labels
2. developing Work in progress enhancement feature: files 🍀 2025-Spring
Projects
📁 Files team
Status: 🏗️ In progress
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.

[App Files] Can't see owner's name if a file/folder is reshared twice
9 participants
@Fenn-CS @nimishavijay @nickvergessen @jancborchardt @susnux @artonge @sorbaugh @AndyScherzinger @skjnldsv