Merge pull request #46448 from nextcloud/bugfix/noid/fix-missing-dashboard-widget-icon

fix(dashboard): Unify widget icon colors and document it's behaviour

Author: AndyScherzinger

apps/dashboard/lib/Controller/DashboardController.php

@@ -15,6 +15,7 @@
 use OCP\AppFramework\Http\Attribute\OpenAPI;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
+use OCP\Dashboard\IIconWidget;
 use OCP\Dashboard\IManager;
 use OCP\Dashboard\IWidget;
 use OCP\EventDispatcher\IEventDispatcher;
@@ -54,6 +55,7 @@ public function index(): TemplateResponse {
 				'id' => $widget->getId(),
 				'title' => $widget->getTitle(),
 				'iconClass' => $widget->getIconClass(),
+				'iconUrl' => $widget instanceof IIconWidget ? $widget->getIconUrl() : '',
 				'url' => $widget->getUrl()
 			];
 		}, $this->dashboardManager->getWidgets());

apps/dashboard/src/DashboardApp.vue

@@ -24,7 +24,12 @@
 					class="panel">
 					<div class="panel--header">
 						<h2>
-							<span :aria-labelledby="`panel-${panels[panelId].id}--header--icon--description`"
+							<img v-if="apiWidgets[panels[panelId].id].icon_url"
+								:alt="apiWidgets[panels[panelId].id].title + ' icon'"
+								:src="apiWidgets[panels[panelId].id].icon_url"
+								aria-hidden="true">
+							<span v-else
+								:aria-labelledby="`panel-${panels[panelId].id}--header--icon--description`"
 								aria-hidden="true"
 								:class="apiWidgets[panels[panelId].id].icon_class"
 								role="img" />
@@ -97,7 +102,11 @@
 							:checked="isActive(panel)"
 							@input="updateCheckbox(panel, $event.target.checked)">
 						<label :for="'panel-checkbox-' + panel.id" :class="{ draggable: isActive(panel) }">
-							<span :class="panel.iconClass" aria-hidden="true" />
+							<img v-if="panel.iconUrl"
+								:alt="panel.title + ' icon'"
+								:src="panel.iconUrl"
+								aria-hidden="true">
+							<span v-else :class="panel.iconClass" aria-hidden="true" />
 							{{ panel.title }}
 						</label>
 					</li>
@@ -554,6 +563,8 @@ export default {
 			overflow: hidden;
 			text-overflow: ellipsis;
 			cursor: grab;
+
+			img,
 			span {
 				background-size: 32px;
 				width: 32px;
@@ -564,6 +575,10 @@ export default {
 				margin-top: -6px;
 				margin-left: 6px;
 			}
+
+			img {
+				filter: var(--background-invert-if-dark);
+			}
 		}
 	}
 
@@ -651,6 +666,7 @@ export default {
 			text-overflow: ellipsis;
 			white-space: nowrap;
 
+			img,
 			span {
 				position: absolute;
 				top: 16px;
@@ -659,6 +675,10 @@ export default {
 				background-size: 24px;
 			}
 
+			img {
+				filter: var(--background-invert-if-dark);
+			}
+
 			&:hover {
 				border-color: var(--color-primary-element);
 			}

apps/user_status/css/user-status-menu.css

@@ -1,10 +1,4 @@
 /*!
  * SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
- *//*!
- * SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
- * SPDX-License-Identifier: AGPL-3.0-or-later
- *//*!
- * SPDX-FileCopyrightText: 2018 Nextcloud GmbH and Nextcloud contributors
- * SPDX-License-Identifier: AGPL-3.0-or-later
- */.icon-user-status{background-image:url("../img/app.svg")}.icon-user-status-dark{background-image:url("../img/app-dark.svg")}/*# sourceMappingURL=user-status-menu.css.map */
+ */.icon-user-status{background-image:url("../img/app.svg")}.icon-user-status-dark{background-image:url("../img/app-dark.svg");filter:var(--background-invert-if-dark)}/*# sourceMappingURL=user-status-menu.css.map */

apps/user_status/css/user-status-menu.css.map

@@ -2,14 +2,11 @@
  * SPDX-FileCopyrightText: 2020 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
  */
-@use 'variables';
-@import 'functions';
-
 .icon-user-status {
 	background-image: url("../img/app.svg");
 }
 
 .icon-user-status-dark {
 	background-image: url("../img/app-dark.svg");
-
+	filter: var(--background-invert-if-dark);
 }

apps/user_status/css/user-status-menu.scss

@@ -25,18 +25,23 @@ class Manager implements IManager {
 	/** @var array<string, IWidget> */
 	private array $widgets = [];
 
-	private ContainerInterface $serverContainer;
 	private ?IAppManager $appManager = null;
 
-	public function __construct(ContainerInterface $serverContainer) {
-		$this->serverContainer = $serverContainer;
+	public function __construct(
+		private ContainerInterface $serverContainer,
+		private LoggerInterface $logger,
+	) {
 	}
 
 	private function registerWidget(IWidget $widget): void {
 		if (array_key_exists($widget->getId(), $this->widgets)) {
 			throw new InvalidArgumentException('Dashboard widget with this id has already been registered');
 		}
 
+		if (!preg_match('/^[a-z][a-z0-9\-_]*$/', $widget->getId())) {
+			$this->logger->debug('Deprecated dashboard widget ID provided: "' . $widget->getId() . '" [ ' . get_class($widget) . ' ]. Please use a-z, 0-9, - and _ only, starting with a-z');
+		}
+
 		$this->widgets[$widget->getId()] = $widget;
 	}
 

dist/dashboard-main.js

@@ -14,7 +14,9 @@
  */
 interface IIconWidget extends IWidget {
 	/**
-	 * Get the absolute url for the widget icon
+	 * Get the absolute url for the widget icon (should be colored black or not have a color)
+	 *
+	 * The icon will be inverted automatically in mobile clients and when using dark mode
 	 *
 	 * @return string
 	 * @since 25.0.0

dist/dashboard-main.js.map

@@ -15,7 +15,12 @@
  */
 interface IWidget {
 	/**
-	 * @return string Unique id that identifies the widget, e.g. the app id
+	 * Get a unique identifier for the widget
+	 *
+	 * To ensure uniqueness, it is recommended to user the app id or start with the
+	 * app id followed by a dash.
+	 *
+	 * @return string Unique id that identifies the widget, e.g. the app id. Only use alphanumeric characters, dash and underscore
 	 * @since 20.0.0
 	 */
 	public function getId(): string;
@@ -33,6 +38,13 @@ public function getTitle(): string;
 	public function getOrder(): int;
 
 	/**
+	 * CSS class that shows the widget icon (should be colored black or not have a color)
+	 *
+	 * The icon will be inverted automatically in mobile clients and when using dark mode.
+	 * Therefore, it is NOT recommended to use a css class that sets the background with:
+	 * `var(--icon-…)` as those will adapt to dark/bright mode in the web and still be inverted
+	 * resulting in a dark icon on dark background.
+	 *
 	 * @return string css class that displays an icon next to the widget title
 	 * @since 20.0.0
 	 */