-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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 Comments Web Client Representation #20267
Comments
had a short discussion in the office here. This will be a nightmare on sorting. Get the last 3 comments, what happens when the parent does not exist in this set of 3 items, how do we display them? What happens when the number of parents is 1000 (all nested), etc.... Just skip it for now.
Also applies to federated shares then? |
Federated shares are not public per se. If we can get it working without much trouble I think it is worth to show them in this case as well. Other opinions? |
Well we have a different set of user's. |
Federated shares are comparable more to internal shares than public shares I’d say. For federated shares I’d definitely expect the comments to show. Not necessarily something for the first step of course – but from a product design standpoint I’m always wary of introducing features which don’t work everywhere as expected (on mobile as well for example). Since we advertise federated sharing, and will advertise comments as well, comments should work for federated shares. And just as a nice background read: How to display threaded discussions on the web |
@blizzz have a look at system tags implementation to see how to define Webdav-compatible backbone collections:
The dav properties mapping is done inside the model: core/core/js/systemtags/systemtagmodel.js Line 29 in 6cb95f4
Also: need to figure out how to make it easier to do a Webdav REPORT call from JS. (I'll have a look at this) |
Comments have been implemented. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Contributes to #16328
@nickvergessen @blizzz @DeepDiver1975 @jancborchardt
The text was updated successfully, but these errors were encountered: