Skip to content

fix(AppManager): Allow to query dark **or** bright icon #44073

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

Merged
nickvergessen merged 1 commit into from
Mar 8, 2024
Merged

fix(AppManager): Allow to query dark **or** bright icon #44073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/updatenotification/lib/Notification/AppUpdateNotifier.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ public function prepare(INotification $notification, string $languageCode): INot
// Prepare translation factory for requested language
$l = $this->l10nFactory->get(Application::APP_NAME, $languageCode);

$icon = $this->appManager->getAppIcon($appId);
$icon = $this->appManager->getAppIcon($appId, true);
if ($icon === null) {
$icon = $this->urlGenerator->imagePath('core', 'default-app-icon');
$icon = $this->urlGenerator->imagePath('core', 'actions/change.svg');
}

$action = $notification->createAction();
Expand Down
4 changes: 2 additions & 2 deletions lib/private/App/AppManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ public function __construct(
) {
}

public function getAppIcon(string $appId): ?string {
$possibleIcons = [$appId . '.svg', 'app.svg', $appId . '-dark.svg', 'app-dark.svg'];
public function getAppIcon(string $appId, bool $dark = false): ?string {
$possibleIcons = $dark ? [$appId . '-dark.svg', 'app-dark.svg'] : [$appId . '.svg', 'app.svg'];
$icon = null;
foreach ($possibleIcons as $iconName) {
try {
Expand Down
3 changes: 2 additions & 1 deletion lib/public/App/IAppManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,11 @@ public function getAppVersion(string $appId, bool $useCache = true): string;
* Returns the app icon or null if none is found
*
* @param string $appId
* @param bool $dark Enable to request a dark icon variant, default is a white icon
* @return string|null
* @since 29.0.0
*/
public function getAppIcon(string $appId): string|null;
public function getAppIcon(string $appId, bool $dark = false): string|null;

/**
* Check if an app is enabled for user
Expand Down
49 changes: 38 additions & 11 deletions tests/lib/App/AppManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,16 @@ protected function setUp(): void {
/**
* @dataProvider dataGetAppIcon
*/
public function testGetAppIcon($callback, string|null $expected) {
public function testGetAppIcon($callback, ?bool $dark, string|null $expected) {
$this->urlGenerator->expects($this->atLeastOnce())
->method('imagePath')
->willReturnCallback($callback);

$this->assertEquals($expected, $this->manager->getAppIcon('test'));
if ($dark !== null) {
$this->assertEquals($expected, $this->manager->getAppIcon('test', $dark));
} else {
$this->assertEquals($expected, $this->manager->getAppIcon('test'));
}
}

public function dataGetAppIcon(): array {
Expand All @@ -162,36 +166,59 @@ public function dataGetAppIcon(): array {
return [
'does not find anything' => [
$nothing,
false,
null,
],
'nothing if request dark but only bright available' => [
$createCallback(['app.svg']),
true,
null,
],
'only app.svg' => [
'nothing if request bright but only dark available' => [
$createCallback(['app-dark.svg']),
false,
null,
],
'bright and only app.svg' => [
$createCallback(['app.svg']),
false,
'/path/app.svg',
],
'only app-dark.svg' => [
'dark and only app-dark.svg' => [
$createCallback(['app-dark.svg']),
true,
'/path/app-dark.svg',
],
'only appname -dark.svg' => [
'dark only appname -dark.svg' => [
$createCallback(['test-dark.svg']),
true,
'/path/test-dark.svg',
],
'only appname.svg' => [
'bright and only appname.svg' => [
$createCallback(['test.svg']),
false,
'/path/test.svg',
],
'priotize custom over default' => [
$createCallback(['app.svg', 'test.svg']),
false,
'/path/test.svg',
],
'priotize default over dark' => [
$createCallback(['test-dark.svg', 'app-dark.svg', 'app.svg']),
'/path/app.svg',
'defaults to bright' => [
$createCallback(['test-dark.svg', 'test.svg']),
null,
'/path/test.svg',
],
'priotize custom over default' => [
$createCallback(['test.svg', 'test-dark.svg', 'app-dark.svg']),
'no dark icon on default' => [
$createCallback(['test-dark.svg', 'test.svg', 'app-dark.svg', 'app.svg']),
false,
'/path/test.svg',
],
'no bright icon on dark' => [
$createCallback(['test-dark.svg', 'test.svg', 'app-dark.svg', 'app.svg']),
true,
'/path/test-dark.svg',
],
];
}

Expand Down