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

Provide tags #1147

Closed
tobiasKaminsky opened this issue Apr 12, 2023 · 15 comments
Closed

Provide tags #1147

tobiasKaminsky opened this issue Apr 12, 2023 · 15 comments

Comments

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Apr 12, 2023

For nextcloud/files-clients#39 it is needed to have tags within activity stream.
Jan wished to have this only on Desktop, but other clients then also shall implement it.

From API side adding "tags": "['tag 1', 'tag 2'] might be the easiest, without breaking anything?

@AndyScherzinger for planning
@artonge @miaulalala for actual work

@nickvergessen
Copy link
Member

nickvergessen commented Apr 12, 2023

I recommend to add a flag to the request to opt-in into this, also that might have quite some bad performance impact if done incorrectly. Should definitely only be populated in post-processing stage, so we can load all tags of all file ids with a single request.

Follow the "previews" parameter from those two methods to see opt-in in action:
https://github.com/nextcloud/activity/blob/master/lib/Controller/APIv2Controller.php#L169-L185

I would implement it before these lines:
https://github.com/nextcloud/activity/blob/master/lib/Controller/APIv2Controller.php#L266-L267
And then "assign" the tags inside the following loop.

@nickvergessen
Copy link
Member

Since it's then used by official clients, it should be documented in https://github.com/nextcloud/activity/blob/master/docs/endpoint-v2.md#parameters as well

@tobiasKaminsky
Copy link
Member Author

tobiasKaminsky commented Apr 13, 2023

@nextcloud/designers
can you provide us with ideas/mockup how tags should be shown?
For one file it is quite obvious, I think.
But what about clustered activities, e.g. "Tobias" has changed test.md, image.jpg and readme.md?

Or shall tags only be shown if activity is only for one file/folder?

@tobiasKaminsky
Copy link
Member Author

image

@nimishavijay
Copy link
Member

I would agree with Tobi, if one file was changed, show tags also
Tobias has added tags Personal and Urgent to file.md

If multiple files were changed:
Tobias has added tags to file.md, test.md and review.md

Additionally if it is possible to find out if the same tags were added to multiple files then they could be shown together
Tobias has added tags Personal and Urgent to file.md, test.md and review.md

@nickvergessen
Copy link
Member

The question was not about when tags are assigned/removed, but how to show combined activity:

User A changed files 1, 2, 3

1 is tagged with 1A, 1B and 1C
2 is tagged with 2A, 2B and 2C
3 is tagged with 3A, 3B and 3C

We can not put those 9 tags in a single subline without giving indication which item is being referred to

@nimishavijay
Copy link
Member

Right, in that case we can say "User A has added tags to files 1, 2, and 3" like I mentioned.

And in case 1, 2, and 3 are all tagged with the same tags A and B, "User A has added tags A and B to 1, 2, and 3" but this is a nice-to-have.

@nickvergessen
Copy link
Member

It's not about the tagging action, it's about show the tags of files that were created/modified.

@nimishavijay
Copy link
Member

Yes, I understand, I mean we don't have to show the individual tags if there are multiple files, like Tobi mentioned.
I think I am misunderstanding something. Maybe @jancborchardt will understand this issue better?

@jancborchardt
Copy link
Member

I recommend to add a flag to the request to opt-in into this, also that might have quite some bad performance impact if done incorrectly.

If there is any performance concern with this then maybe it makes more sense to not do it in Activity (i.e. the desktop tray menu) but only in the details view (which is only one click away)?

So we do have the tags in file listings (like mobile apps and web view) but since the desktop app is an activity list, it does not need it.

@tobiasKaminsky
Copy link
Member Author

From my side it is not about performance, but about how to show tags in case of multiple files.
Please check above discussion, starting there #1147 (comment)

@AndyScherzinger
Copy link
Member

I simply wouldn't show tags for activities on files/folders except if a file/folder got tagged, then show that as an activity. I don't think it adds extra value to the activity feed. Just my opinion.

@tobiasKaminsky
Copy link
Member Author

@jancborchardt please clarify, as this was your idea/wish :-)

@jancborchardt
Copy link
Member

Yep, after thinking about it more, considering possinle designs and drawbacks I have to agree with @AndyScherzinger.

And as I said above, resoning:

So we do have the tags in file listings (like mobile apps and web view) but since the desktop app is an activity list, it does not need it.

Of course for activities which are for tagging / removing tags, those should show in the stream.

@tobiasKaminsky
Copy link
Member Author

This is already the case:
image

So nothing to do :)

@nickvergessen nickvergessen closed this as not planned Won't fix, can't repro, duplicate, stale Apr 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants