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.

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

Jump to bottom

Fix primary color usage in emails and federation #36348

Merged
merged 3 commits into from
Jan 30, 2023
Merged

Fix primary color usage in emails and federation #36348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a192641
Use the default color and matching text color for the federation invite
nickvergessen Jan 25, 2023
79b8c68
Use the default primary color instead of "randomly" the color of the …
nickvergessen Jan 25, 2023
959e2aa
fix(tests): Fix method name in unit tests
nickvergessen Jan 26, 2023
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
36 changes: 13 additions & 23 deletions apps/federatedfilesharing/lib/Settings/Personal.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<?php

declare(strict_types=1);

/**
* @copyright Copyright (c) 2017 Arthur Schiwon <blizzz@arthur-schiwon.de>
*
Expand Down Expand Up @@ -29,30 +32,27 @@
use OCA\FederatedFileSharing\FederatedShareProvider;
use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Services\IInitialState;
use OCP\IL10N;
use OCP\Defaults;
use OCP\IUserSession;
use OCP\IURLGenerator;
use OCP\Settings\ISettings;

class Personal implements ISettings {
private FederatedShareProvider $federatedShareProvider;
private IUserSession $userSession;
private IL10N $l;
private \OC_Defaults $defaults;
private Defaults $defaults;
private IInitialState $initialState;
private IURLGenerator $urlGenerator;

public function __construct(
FederatedShareProvider $federatedShareProvider, #
FederatedShareProvider $federatedShareProvider,
IUserSession $userSession,
IL10N $l,
\OC_Defaults $defaults,
Defaults $defaults,
IInitialState $initialState,
IURLGenerator $urlGenerator
) {
$this->federatedShareProvider = $federatedShareProvider;
$this->userSession = $userSession;
$this->l = $l;
$this->defaults = $defaults;
$this->initialState = $initialState;
$this->urlGenerator = $urlGenerator;
Expand All @@ -62,35 +62,25 @@ public function __construct(
* @return TemplateResponse returns the instance with all parameters set, ready to be rendered
* @since 9.1
*/
public function getForm() {
public function getForm(): TemplateResponse {
$cloudID = $this->userSession->getUser()->getCloudId();
$url = 'https://nextcloud.com/sharing#' . $cloudID;

$parameters = [
'message_with_URL' => $this->l->t('Share with me through my #Nextcloud Federated Cloud ID, see %s', [$url]),
'message_without_URL' => $this->l->t('Share with me through my #Nextcloud Federated Cloud ID', [$cloudID]),
'logoPath' => $this->defaults->getLogo(),
'reference' => $url,
'cloudId' => $cloudID,
'color' => $this->defaults->getColorPrimary(),
'textColor' => "#ffffff",
];

$this->initialState->provideInitialState('color', $this->defaults->getColorPrimary());
$this->initialState->provideInitialState('textColor', '#fffff');
$this->initialState->provideInitialState('color', $this->defaults->getDefaultColorPrimary());
$this->initialState->provideInitialState('textColor', $this->defaults->getDefaultTextColorPrimary());
$this->initialState->provideInitialState('logoPath', $this->defaults->getLogo());
$this->initialState->provideInitialState('reference', $url);
$this->initialState->provideInitialState('cloudId', $cloudID);
$this->initialState->provideInitialState('docUrlFederated', $this->urlGenerator->linkToDocs('user-sharing-federated'));

return new TemplateResponse('federatedfilesharing', 'settings-personal', $parameters, '');
return new TemplateResponse('federatedfilesharing', 'settings-personal', [], TemplateResponse::RENDER_AS_BLANK);
}

/**
* @return string the section ID, e.g. 'sharing'
* @since 9.1
*/
public function getSection() {
public function getSection(): ?string {
if ($this->federatedShareProvider->isIncomingServer2serverShareEnabled() ||
$this->federatedShareProvider->isIncomingServer2serverGroupShareEnabled()) {
return 'sharing';
Expand All @@ -106,7 +96,7 @@ public function getSection() {
* E.g.: 70
* @since 9.1
*/
public function getPriority() {
public function getPriority(): int {
return 40;
}
}
9 changes: 9 additions & 0 deletions apps/theming/lib/ThemingDefaults.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,15 @@ public function getTextColorPrimary() {
return $this->util->invertTextColor($this->getColorPrimary()) ? '#000000' : '#ffffff';
}

/**
* Color of text in the header and primary buttons
*
* @return string
*/
public function getDefaultTextColorPrimary() {
return $this->util->invertTextColor($this->getDefaultColorPrimary()) ? '#000000' : '#ffffff';
}

/**
* Has the admin disabled user customization
*/
Expand Down
6 changes: 3 additions & 3 deletions lib/private/Mail/EMailTemplate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ public function addHeader() {
$this->headerAdded = true;

$logoUrl = $this->urlGenerator->getAbsoluteURL($this->themingDefaults->getLogo(false));
$this->htmlBody .= vsprintf($this->header, [$this->themingDefaults->getColorPrimary(), $logoUrl, $this->themingDefaults->getName()]);
$this->htmlBody .= vsprintf($this->header, [$this->themingDefaults->getDefaultColorPrimary(), $logoUrl, $this->themingDefaults->getName()]);
}

/**
Expand Down Expand Up @@ -555,7 +555,7 @@ public function addBodyButtonGroup(string $textLeft,
$this->ensureBodyIsOpened();
$this->ensureBodyListClosed();

$color = $this->themingDefaults->getColorPrimary();
$color = $this->themingDefaults->getDefaultColorPrimary();
$textColor = $this->themingDefaults->getTextColorPrimary();

$this->htmlBody .= vsprintf($this->buttonGroup, [$color, $color, $urlLeft, $color, $textColor, $textColor, $textLeft, $urlRight, $textRight]);
Expand Down Expand Up @@ -586,7 +586,7 @@ public function addBodyButton(string $text, string $url, $plainText = '') {
$text = htmlspecialchars($text);
}

$color = $this->themingDefaults->getColorPrimary();
$color = $this->themingDefaults->getDefaultColorPrimary();
$textColor = $this->themingDefaults->getTextColorPrimary();
$this->htmlBody .= vsprintf($this->button, [$color, $color, $url, $color, $textColor, $textColor, $text]);

Expand Down
24 changes: 24 additions & 0 deletions lib/public/Defaults.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,18 @@ public function getColorPrimary(): string {
return $this->defaults->getColorPrimary();
}

/**
* Return the default color primary
* @return string
* @since 25.0.4
*/
public function getDefaultColorPrimary(): string {
if (method_exists($this->defaults, 'getDefaultColorPrimary')) {
return $this->defaults->getDefaultColorPrimary();
}
return $this->defaults->getColorPrimary();
}
Comment on lines +213 to +218
Copy link
Member

Choose a reason for hiding this comment

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

getDefaultColorPrimary should always be available no?

Copy link
Member Author

Choose a reason for hiding this comment

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

OC_Defaults only has getColorPrimary and I'm unwilling to debug which version of the defaults object is used where and why.

server/lib/private/legacy/OC_Defaults.php

Lines 301 to 309 in 3860dad

public function getColorPrimary() {
if ($this->themeExist('getColorPrimary')) {
return $this->theme->getColorPrimary();
}
if ($this->themeExist('getMailHeaderColor')) {
return $this->theme->getMailHeaderColor();
}
return $this->defaultColorPrimary;
}

That's a cleanup task for another day.

Copy link
Member

Choose a reason for hiding this comment

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

ok then


/**
* @param string $key
* @return string URL to doc with key
Expand All @@ -231,4 +243,16 @@ public function getTitle(): string {
public function getTextColorPrimary(): string {
return $this->defaults->getTextColorPrimary();
}

/**
* Returns primary color
* @return string
* @since 25.0.4
*/
public function getDefaultTextColorPrimary(): string {
if (method_exists($this->defaults, 'getDefaultTextColorPrimary')) {
return $this->defaults->getDefaultTextColorPrimary();
}
return $this->defaults->getTextColorPrimary();
}
}
8 changes: 4 additions & 4 deletions tests/lib/Mail/EMailTemplateTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ protected function setUp(): void {
public function testEMailTemplateCustomFooter() {
$this->defaults
->expects($this->any())
->method('getColorPrimary')
->method('getDefaultColorPrimary')
->willReturn('#0082c9');
$this->defaults
->expects($this->any())
Expand Down Expand Up @@ -104,7 +104,7 @@ public function testEMailTemplateCustomFooter() {
public function testEMailTemplateDefaultFooter() {
$this->defaults
->expects($this->any())
->method('getColorPrimary')
->method('getDefaultColorPrimary')
->willReturn('#0082c9');
$this->defaults
->expects($this->any())
Expand Down Expand Up @@ -147,7 +147,7 @@ public function testEMailTemplateDefaultFooter() {
public function testEMailTemplateSingleButton() {
$this->defaults
->expects($this->any())
->method('getColorPrimary')
->method('getDefaultColorPrimary')
->willReturn('#0082c9');
$this->defaults
->expects($this->any())
Expand Down Expand Up @@ -192,7 +192,7 @@ public function testEMailTemplateSingleButton() {
public function testEMailTemplateAlternativePlainTexts() {
$this->defaults
->expects($this->any())
->method('getColorPrimary')
->method('getDefaultColorPrimary')
->willReturn('#0082c9');
$this->defaults
->expects($this->any())
Expand Down