Skip to content

Commit

Permalink
feat(NavigationManager): Always sort the default app first
Browse files Browse the repository at this point in the history 
Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
@susnux
susnux committed Oct 19, 2023
1 parent 08cff07 commit 363d9eb
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 16 deletions.
8 changes: 4 additions & 4 deletions lib/private/App/AppManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -823,9 +823,9 @@ public function getDefaultEnabledApps():array {
return $this->defaultEnabled;
}

public function getDefaultAppForUser(?IUser $user = null): string {
public function getDefaultAppForUser(?IUser $user = null, bool $withFallbacks = true): string {
// Set fallback to always-enabled files app
$appId = 'files';
$appId = $withFallbacks ? 'files' : '';
$defaultApps = explode(',', $this->config->getSystemValueString('defaultapp', ''));
$defaultApps = array_filter($defaultApps);

Expand All @@ -834,7 +834,7 @@ public function getDefaultAppForUser(?IUser $user = null): string {
if ($user !== null) {
$userDefaultApps = explode(',', $this->config->getUserValue($user->getUID(), 'core', 'defaultapp'));
$defaultApps = array_filter(array_merge($userDefaultApps, $defaultApps));
if (empty($defaultApps)) {
if (empty($defaultApps) && $withFallbacks) {
/* Fallback on user defined apporder */
$customOrders = json_decode($this->config->getUserValue($user->getUID(), 'core', 'apporder', '[]'), true, flags:JSON_THROW_ON_ERROR);
if (!empty($customOrders)) {
Expand All @@ -845,7 +845,7 @@ public function getDefaultAppForUser(?IUser $user = null): string {
}
}

if (empty($defaultApps)) {
if (empty($defaultApps) && $withFallbacks) {
$defaultApps = ['dashboard','files'];
}

Expand Down
46 changes: 40 additions & 6 deletions lib/private/NavigationManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,26 +123,44 @@ public function getAll(string $type = 'link'): array {
});
}

return $this->proceedNavigation($result);
return $this->proceedNavigation($result, $type);
}

/**
* Sort navigation entries by order, name and set active flag
* Sort navigation entries default app is always sorted first, then by order, name and set active flag
*
* @param array $list
* @return array
*/
private function proceedNavigation(array $list): array {
private function proceedNavigation(array $list, string $type): array {
uasort($list, function ($a, $b) {
if (isset($a['order']) && isset($b['order'])) {
if (($a['default'] ?? false) xor ($b['default'] ?? false)) {
// Always sort the default app first
return ($a['default'] ?? false) ? -1 : 1;
} elseif (isset($a['order']) && isset($b['order'])) {
// Sort by order
return ($a['order'] < $b['order']) ? -1 : 1;
} elseif (isset($a['order']) || isset($b['order'])) {
// Sort the one that has an order property first
return isset($a['order']) ? -1 : 1;
} else {
// Sort by name otherwise
return ($a['name'] < $b['name']) ? -1 : 1;
}
});

if ($type === 'all' || $type === 'link') {
// There might be the case that no default app was set, in this case the first app is the default app.
// Otherwise the default app is already the ordered first, so setting the default prop will make no difference.
foreach ($list as $index => &$navEntry) {
if ($navEntry['type'] === 'link') {
$navEntry['default'] = true;
break;
}
}
unset($navEntry);
}

$activeApp = $this->getActiveEntry();
if ($activeApp !== null) {
foreach ($list as $index => &$navEntry) {
Expand Down Expand Up @@ -293,6 +311,8 @@ private function init() {
$customOrders = [];
}

// The default app of the current user without fallbacks
$defaultApp = $this->appManager->getDefaultAppForUser($this->userSession->getUser(), false);

foreach ($apps as $app) {
if (!$this->userSession->isLoggedIn() && !$this->appManager->isEnabledForUser($app, $this->userSession->getUser())) {
Expand Down Expand Up @@ -335,14 +355,28 @@ private function init() {
$icon = $this->urlGenerator->imagePath('core', 'default-app-icon');
}

$this->add([
$this->add(array_merge([
// Navigation id
'id' => $id,
// Order where this entry should be shown
'order' => $order,
// Target of the navigation entry
'href' => $route,
// The icon used for the naviation entry
'icon' => $icon,
// Type of the navigation entry ('link' vs 'settings')
'type' => $type,
// Localized name of the navigation entry
'name' => $l->t($nav['name']),
]);
], $type === 'link' ? [
// This is the default app that will always be shown first
'default' => $defaultApp === $id,
// App that registered this navigation entry (not necessarly the same as the id)
'app' => $app,
// The key used to identify this entry in the navigations entries
'key' => $key,
] : []
));
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion lib/public/App/IAppManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,13 @@ public function getAppRestriction(string $appId): array;
*
* If `user` is not passed, the currently logged in user will be used
*
* @param ?IUser $user User to query default app for
* @param bool $withFallbacks Include fallback values if no default app was configured manually
*
* @since 25.0.6
* @since 28.0.0 Added optional $withFallbacks parameter
*/
public function getDefaultAppForUser(?IUser $user = null): string;
public function getDefaultAppForUser(?IUser $user = null, bool $withFallbacks = true): string;

/**
* Get the global default apps with fallbacks
Expand Down
61 changes: 56 additions & 5 deletions tests/lib/NavigationManagerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function addArrayData() {
//'icon' => 'optional',
'href' => 'url',
'active' => true,
'unread' => 0
'unread' => 0,
],
'entry id2' => [
'id' => 'entry id',
Expand All @@ -106,7 +106,8 @@ public function addArrayData() {
'active' => false,
'type' => 'link',
'classes' => '',
'unread' => 0
'unread' => 0,
'default' => true,
]
]
];
Expand Down Expand Up @@ -349,7 +350,10 @@ public function providesNavigationConfig() {
'active' => false,
'type' => 'link',
'classes' => '',
'unread' => 0
'unread' => 0,
'default' => true,
'app' => 'test',
'key' => 0,
]],
['logout' => $defaults['logout']]
),
Expand All @@ -372,7 +376,7 @@ public function providesNavigationConfig() {
'active' => false,
'type' => 'settings',
'classes' => '',
'unread' => 0
'unread' => 0,
]],
['logout' => $defaults['logout']]
),
Expand All @@ -382,6 +386,47 @@ public function providesNavigationConfig() {
],
]]
],
'with-multiple' => [
array_merge(
['accessibility_settings' => $defaults['accessibility_settings']],
['settings' => $defaults['settings']],
['test' => [
'id' => 'test',
'order' => 100,
'href' => '/apps/test/',
'icon' => '/apps/test/img/app.svg',
'name' => 'Test',
'active' => false,
'type' => 'link',
'classes' => '',
'unread' => 0,
'default' => false,
'app' => 'test',
'key' => 0,
],
'test1' => [
'id' => 'test1',
'order' => 50,
'href' => '/apps/test/',
'icon' => '/apps/test/img/app.svg',
'name' => 'Other test',
'active' => false,
'type' => 'link',
'classes' => '',
'unread' => 0,
'default' => true, // because of order
'app' => 'test',
'key' => 1,
]],
['logout' => $defaults['logout']]
),
['navigations' => [
'navigation' => [
['route' => 'test.page.index', 'name' => 'Test'],
['route' => 'test.page.index', 'name' => 'Other test', 'order' => 50],
]
]]
],
'admin' => [
array_merge(
$adminSettings,
Expand All @@ -395,7 +440,10 @@ public function providesNavigationConfig() {
'active' => false,
'type' => 'link',
'classes' => '',
'unread' => 0
'unread' => 0,
'default' => true,
'app' => 'test',
'key' => 0,
]],
['logout' => $defaults['logout']]
),
Expand Down Expand Up @@ -448,6 +496,9 @@ public function testWithAppManagerAndApporder() {
'active' => false,
'classes' => '',
'unread' => 0,
'default' => true,
'app' => 'test',
'key' => 0,
],
];
$navigation = ['navigations' => [
Expand Down

0 comments on commit 363d9eb

Please sign in to comment.