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

[stable29] fix(dashboard): Unify widget icon colors and document it's behaviour #46626

Merged
merged 4 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions apps/dashboard/lib/Controller/DashboardController.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
use OCP\AppFramework\Http\JSONResponse;
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;
Expand Down Expand Up @@ -75,6 +76,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());
Expand Down
24 changes: 22 additions & 2 deletions apps/dashboard/src/DashboardApp.vue
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,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" />
Expand Down Expand Up @@ -93,7 +98,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>
Expand Down Expand Up @@ -546,6 +555,8 @@ export default {
overflow: hidden;
text-overflow: ellipsis;
cursor: grab;

img,
span {
background-size: 32px;
width: 32px;
Expand All @@ -556,6 +567,10 @@ export default {
margin-top: -6px;
margin-left: 6px;
}

img {
filter: var(--background-invert-if-dark);
}
}
}

Expand Down Expand Up @@ -643,6 +658,7 @@ export default {
text-overflow: ellipsis;
white-space: nowrap;

img,
span {
position: absolute;
top: 16px;
Expand All @@ -651,6 +667,10 @@ export default {
background-size: 24px;
}

img {
filter: var(--background-invert-if-dark);
}

&:hover {
border-color: var(--color-primary-element);
}
Expand Down
2 changes: 1 addition & 1 deletion apps/user_status/css/user-status-menu.css
Original file line number Diff line number Diff line change
@@ -1 +1 @@
.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 */
2 changes: 1 addition & 1 deletion apps/user_status/css/user-status-menu.css.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions apps/user_status/css/user-status-menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,11 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/
@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);
}
4 changes: 2 additions & 2 deletions dist/core-common.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/core-common.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions dist/dashboard-main.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/dashboard-main.js.map

Large diffs are not rendered by default.

11 changes: 8 additions & 3 deletions lib/private/Dashboard/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,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;
}

Expand Down
4 changes: 3 additions & 1 deletion lib/public/Dashboard/IIconWidget.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,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
Expand Down
14 changes: 13 additions & 1 deletion lib/public/Dashboard/IWidget.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,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;
Expand All @@ -50,6 +55,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
*/
Expand Down
Loading