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

feature: added new FavouriteWidget to display favorite files in dashboard widget #49534

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

Conversation

yemkareems
Copy link
Contributor

feature: added new FavouriteWidget to display favorite files in dashboard widget

added a new widget to display favorite files in dashboard and to show a link to more favorites to apps/files/favorites. if max count of favorites of 50 is reached then also 'No favorites' is displayed

image

fixes: #23308

  • Resolves: #

Summary

TODO

  • ...

Checklist

@yemkareems yemkareems added the 3. to review Waiting for reviews label Nov 28, 2024
@yemkareems yemkareems self-assigned this Nov 28, 2024
@yemkareems yemkareems force-pushed the feature/23308/create-new-favorite-dashboard-widget branch from 09e1e7c to 341e377 Compare November 28, 2024 07:18
@@ -7,14 +7,14 @@
class ComposerStaticInitFiles
Copy link
Contributor

Choose a reason for hiding this comment

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

There is an issue in this file, please run build/autoloaderchecker.sh and commit the result without modifications

Comment on lines +31 to +59
private IUserSession $userSession;
private IL10N $l10n;
private IURLGenerator $urlGenerator;
private IMimeTypeDetector $mimeTypeDetector;
private IUserManager $userManager;
private ITagManager $tagManager;
private IRootFolder $rootFolder;
private IPreview $previewManager;
public const FAVORITE_LIMIT = 50;

public function __construct(
IUserSession $userSession,
IL10N $l10n,
IURLGenerator $urlGenerator,
IMimeTypeDetector $mimeTypeDetector,
IUserManager $userManager,
ITagManager $tagManager,
IRootFolder $rootFolder,
IPreview $previewManager,
) {
$this->userSession = $userSession;
$this->l10n = $l10n;
$this->urlGenerator = $urlGenerator;
$this->mimeTypeDetector = $mimeTypeDetector;
$this->userManager = $userManager;
$this->tagManager = $tagManager;
$this->rootFolder = $rootFolder;
$this->previewManager = $previewManager;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private IUserSession $userSession;
private IL10N $l10n;
private IURLGenerator $urlGenerator;
private IMimeTypeDetector $mimeTypeDetector;
private IUserManager $userManager;
private ITagManager $tagManager;
private IRootFolder $rootFolder;
private IPreview $previewManager;
public const FAVORITE_LIMIT = 50;
public function __construct(
IUserSession $userSession,
IL10N $l10n,
IURLGenerator $urlGenerator,
IMimeTypeDetector $mimeTypeDetector,
IUserManager $userManager,
ITagManager $tagManager,
IRootFolder $rootFolder,
IPreview $previewManager,
) {
$this->userSession = $userSession;
$this->l10n = $l10n;
$this->urlGenerator = $urlGenerator;
$this->mimeTypeDetector = $mimeTypeDetector;
$this->userManager = $userManager;
$this->tagManager = $tagManager;
$this->rootFolder = $rootFolder;
$this->previewManager = $previewManager;
}
public function __construct(
private IIUserSession $userSession,
private IIL10N $l10n,
private IIURLGenerator $urlGenerator,
private IIMimeTypeDetector $mimeTypeDetector,
private IIUserManager $userManager,
private IITagManager $tagManager,
private IIRootFolder $rootFolder,
private IIPreview $previewManager,
) {
}

Use promoted properties on new classes.

use OCP\IUserManager;
use OCP\IUserSession;

class FavouriteWidget implements IWidget, IIconWidget, IAPIWidget, IAPIWidgetV2, IButtonWidget {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are all these interfaces required? IWidget is inherited anyway, and for apiwidget are both v1 and v2 needed? I never touched dashboard widgets before, asking to be sure.

}

public function getId(): string {
return Application::APP_ID;
Copy link
Contributor

Choose a reason for hiding this comment

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

We may have other files related widget later no?

Suggested change
return Application::APP_ID;
return Application::APP_ID.'-favourites';

}

public function getTitle(): string {
return $this->l10n->t('Favorites');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it Favorite ou Favourite? We should be consistent in code and UI and filenames.

}

public function getUrl(): ?string {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should link to favorites view, no?

return;
}
return;
//Util::addScript(Application::APP_ID, 'recommendations-dashboard');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//Util::addScript(Application::APP_ID, 'recommendations-dashboard');

Comment on lines +86 to +90
$user = $this->userSession->getUser();
if ($user === null) {
return;
}
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

That’s not doing anything?

Comment on lines +104 to +106
} elseif (isset($favorites[self::FAVORITE_LIMIT])) {
return [];
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is that returning an empty list?

@AndyScherzinger
Copy link
Member

Looping in @jancborchardt for

if max count of favorites of 50 is reached then also 'No favorites' is displayed

This seems unintuitive since "no favorites" implies count==0 but it is actually count>=50. So I would expect something like "too many favorites to display" or show the 50 and a "more" element/button.

Or I misunderstand the description @yemkareems 😄

@skjnldsv
Copy link
Member

Following Andy's comment, I would actually say we show the last xx updated favorites (like 20 🤷 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Favourites files to dashboard
4 participants