Feature: share articles (front-end part)

[mnassabain]

Part 2 (front-end) of #1191

[codecov-io]

Codecov Report

Merging #1217 (47eba7c) into master (62ca5f3) will increase coverage by 0.08%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1217      +/-   ##
============================================
+ Coverage     87.19%   87.28%   +0.08%     
  Complexity      701      701              
============================================
  Files            60       60              
  Lines          2514     2508       -6     
============================================
- Hits           2192     2189       -3     
+ Misses          322      319       -3     

Impacted Files	Coverage Δ	Complexity Δ
lib/Fetcher/FeedFetcher.php	88.57% <0.00%> (-0.17%)	37.00% <0.00%> (ø%)
lib/Service/FeedServiceV2.php	100.00% <0.00%> (ø)	28.00% <0.00%> (ø%)
lib/Command/Debug/ItemList.php	0.00% <0.00%> (ø)	6.00% <0.00%> (ø%)
lib/Command/Debug/FeedItemList.php	0.00% <0.00%> (ø)	6.00% <0.00%> (ø%)
lib/Command/Debug/FolderItemList.php	0.00% <0.00%> (ø)	8.00% <0.00%> (ø%)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62ca5f3...47eba7c. Read the comment docs.

[mnassabain]

I've updated our front-end changes to match the new back-end implementation.
The code is mergeable with the back-end branch and sharing is functional.
We'll be watching for your feedback! Thanks

[anoymouserver]

Some notes after playing around a bit:

• clicking the sharing button in the compact view breaks the entire layout
  
• the user selection should use the display name for displaying and searching like e.g. the files app does
  ->
• successfully sharing an item should close the dialog (currently you have to click the sharing button again to close it)
• if the feed item does not have any author info, the dash (-) is probably unnecessary
  
• the username after shared by is clickable but doesn't lead you anywhere. Also the users display name should be used here instead.

[mnassabain]

@anoymouserver thank you for the detailed analysis. We are working on correcting these issues.

However I have two questions:

1. Could you tell me what dimensions/configurations are you using to get the compact view? I cannot seem to reproduce the bug you have.
2. Is it a good idea to close the dialog after sharing? It could be annoying to the user if he wanted to share an item with more people. Maybe a better idea would be to be able to close the dialog by clicking somewhere else on the screen, and not necessarily on the dropdown button?

Thanks

Update: we are looking into using the nextcloud popover menu, I think it'll solve the style problems

[anoymouserver]

1. The 'compact view' is a separate view which could be enabled in the 'Settings' menu at the bottom left (2nd checkbox).
   In general I'm using NC 20.0.8 with your two branches merged together. The display is FullHD, so the viewport is only reduced by the browser and task bar.
2. If it doesn't close after sharing it should at least close if you click somewhere else (as you also suggested), I'm not sure how dropdowns for options are handled generally in NC.
   This whole frontend part will probably also be redesigned and unified once the frontend rewrite is done, but until then it should be as usable as possible and I'm not sure how often it happens that an items is shared multiple time in a row (I'm using NC News on my own server without any other accounts, so I'm not representative)

[mnassabain]

In that case we will try to fix the existing dropdown, and integrate at least the changes that you've suggested.

One more thing, in order to display "shared by displayname", the display name needs to be stored in the sharedBy field? But I think for some future features it's important to keep the user id in sharedBy. Should we add a new field to store the sharedBy display name? Or would there be a way to update the Item->jsonSerialize() function to convert the user id into the display name?

[anoymouserver]

I don't know how you could get the display name from the JS but storing it in the sharedBy field is not a good idea. This should always be the actual account name, as it is globally unique and used to identify users everywhere .. and the display name could change any time, so storing it is also not an option. Maybe you could provide it via the controller, but that's only a guess.

The user's current display could be requested in the backend code using

// use OCP\IUserManager;
$user = $this->userManager->get($userId);
$displayName = $user->getDisplayName();

[mnassabain]

All right, thanks for the tip! I'll look into it

[mnassabain]

To do

• Localize e-mail share strings 'Check out this site' et l'autre
• Show user display name in share dropdown
• Dialog now closes when user clicks on other part of screen
• Hide dash if an article is shared and without author (-)
• Make username in "shared by ..." unclickable
• "shared by [username]" => "shared by [display name]
• Fix compact view display glitch
• Integrate socialsharing apps: show/hide sharing on social media depending on whether the corresponding socialsharing app is enabled/disabled
• Cleanup + refactor
• Handle share item error response - display ❌ when failed
• Fix bug user not found if exact username is entered

[mnassabain]

All issues from the previous comments were fixed + some other ones that we've found. We've even sleightly refactored the code to tidy it up.
Let us know what you think! Thanks!

PS. I'd like to compliment you @SMillerDev and @anoymouserver on you reactiveness, your comments have been very helpful and we're very grateful for that!

[anoymouserver]

It looks quite good, nice job!

One further note .. I would hide the social media section if there are no available share apps.

[mnassabain]

Good point, I've fixed it so the share on social media part is hidden when there are no social sharing apps enabled.
Also I've cleaned the css up a bit more

[anoymouserver]

Looks good to me .. frontend works as expected 👍

[SMillerDev]

And can you rebase this one?

[SMillerDev]

Thanks everyone who worked on the feature!

[mnassabain]

Thank you @SMillerDev, @anoymouserver and @Grotax for your help!
Working on this app has been a great experience and we're glad to have contributed to it!