From 407e780147e60726754e9a20477b1809dead0de7 Mon Sep 17 00:00:00 2001 From: Faraz Samapoor Date: Thu, 2 May 2024 11:37:28 +0330 Subject: [PATCH] refactor: Improves readability mainly by utilizing PHP8's constructor property promotion feature. Signed-off-by: Faraz Samapoor --- apps/theming/lib/Capabilities.php | 36 ++---- apps/theming/lib/ITheme.php | 1 - apps/theming/lib/IconBuilder.php | 44 ++----- apps/theming/lib/ImageManager.php | 23 ++-- .../lib/Themes/DarkHighContrastTheme.php | 1 - apps/theming/lib/Themes/DarkTheme.php | 1 - apps/theming/lib/Themes/DefaultTheme.php | 36 ++---- apps/theming/lib/Themes/DyslexiaFont.php | 3 +- apps/theming/lib/Themes/HighContrastTheme.php | 1 - apps/theming/lib/Themes/LightTheme.php | 1 - apps/theming/lib/ThemingDefaults.php | 117 +++++------------- apps/theming/lib/Util.php | 61 +++------ 12 files changed, 87 insertions(+), 238 deletions(-) diff --git a/apps/theming/lib/Capabilities.php b/apps/theming/lib/Capabilities.php index b0c6a71731b86..b6020051b4e1f 100644 --- a/apps/theming/lib/Capabilities.php +++ b/apps/theming/lib/Capabilities.php @@ -41,33 +41,13 @@ * @package OCA\Theming */ class Capabilities implements IPublicCapability { - - /** @var ThemingDefaults */ - protected $theming; - - /** @var Util */ - protected $util; - - /** @var IURLGenerator */ - protected $url; - - /** @var IConfig */ - protected $config; - - protected IUserSession $userSession; - - /** - * @param ThemingDefaults $theming - * @param Util $util - * @param IURLGenerator $url - * @param IConfig $config - */ - public function __construct(ThemingDefaults $theming, Util $util, IURLGenerator $url, IConfig $config, IUserSession $userSession) { - $this->theming = $theming; - $this->util = $util; - $this->url = $url; - $this->config = $config; - $this->userSession = $userSession; + public function __construct( + protected ThemingDefaults $theming, + protected Util $util, + protected IURLGenerator $url, + protected IConfig $config, + protected IUserSession $userSession, + ) { } /** @@ -92,7 +72,7 @@ public function __construct(ThemingDefaults $theming, Util $util, IURLGenerator * }, * } */ - public function getCapabilities() { + public function getCapabilities(): array { $color = $this->theming->getDefaultColorPrimary(); // Same as in DefaultTheme if ($color === BackgroundService::DEFAULT_COLOR) { diff --git a/apps/theming/lib/ITheme.php b/apps/theming/lib/ITheme.php index 4ff455005a2c0..5b3ab6b726676 100644 --- a/apps/theming/lib/ITheme.php +++ b/apps/theming/lib/ITheme.php @@ -30,7 +30,6 @@ * @since 25.0.0 */ interface ITheme { - public const TYPE_THEME = 1; public const TYPE_FONT = 2; diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index 8d6546cdcce33..a878704344aa5 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -31,35 +31,18 @@ use OCP\Files\SimpleFS\ISimpleFile; class IconBuilder { - /** @var ThemingDefaults */ - private $themingDefaults; - /** @var Util */ - private $util; - /** @var ImageManager */ - private $imageManager; - - /** - * IconBuilder constructor. - * - * @param ThemingDefaults $themingDefaults - * @param Util $util - * @param ImageManager $imageManager - */ public function __construct( - ThemingDefaults $themingDefaults, - Util $util, - ImageManager $imageManager + private ThemingDefaults $themingDefaults, + private Util $util, + private ImageManager $imageManager, ) { - $this->themingDefaults = $themingDefaults; - $this->util = $util; - $this->imageManager = $imageManager; } /** * @param $app string app name * @return string|false image blob */ - public function getFavicon($app) { + public function getFavicon($app): bool|string { if (!$this->imageManager->shouldReplaceIcons()) { return false; } @@ -102,7 +85,7 @@ public function getFavicon($app) { * @param $app string app name * @return string|false image blob */ - public function getTouchIcon($app) { + public function getTouchIcon($app): bool|string { try { $icon = $this->renderAppIcon($app, 512); if ($icon === false) { @@ -125,11 +108,8 @@ public function getTouchIcon($app) { * @param $size int size of the icon in px * @return Imagick|false */ - public function renderAppIcon($app, $size) { + public function renderAppIcon($app, $size): Imagick|bool { $appIcon = $this->util->getAppIcon($app); - if ($appIcon === false) { - return false; - } if ($appIcon instanceof ISimpleFile) { $appIconContent = $appIcon->getContent(); $mime = $appIcon->getMimeType(); @@ -150,8 +130,8 @@ public function renderAppIcon($app, $size) { '' . ''; // resize svg magic as this seems broken in Imagemagick - if ($mime === "image/svg+xml" || substr($appIconContent, 0, 4) === "".$appIconContent; } else { $svg = $appIconContent; @@ -163,11 +143,7 @@ public function renderAppIcon($app, $size) { $res = $tmp->getImageResolution(); $tmp->destroy(); - if ($x > $y) { - $max = $x; - } else { - $max = $y; - } + $max = max($x, $y); // convert svg to resized image $appIconFile = new Imagick(); @@ -227,7 +203,7 @@ public function renderAppIcon($app, $size) { * @param $image string relative path to svg file in app directory * @return string|false content of a colorized svg file */ - public function colorSvg($app, $image) { + public function colorSvg($app, $image): bool|string { $imageFile = $this->util->getAppImage($app, $image); if ($imageFile === false || $imageFile === "") { return false; diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php index 994e3f35118f9..c7a47b7fc037f 100644 --- a/apps/theming/lib/ImageManager.php +++ b/apps/theming/lib/ImageManager.php @@ -71,15 +71,11 @@ public function getImageUrl(string $key): string { return $this->urlGenerator->linkToRoute('theming.Theming.getImage', [ 'key' => $key ]) . '?v=' . $cacheBusterCounter; } - switch ($key) { - case 'logo': - case 'logoheader': - case 'favicon': - return $this->urlGenerator->imagePath('core', 'logo/logo.png') . '?v=' . $cacheBusterCounter; - case 'background': - return $this->urlGenerator->linkTo(Application::APP_ID, 'img/background/' . BackgroundService::DEFAULT_BACKGROUND_IMAGE); - } - return ''; + return match ($key) { + 'logo', 'logoheader', 'favicon' => $this->urlGenerator->imagePath('core', 'logo/logo.png') . '?v=' . $cacheBusterCounter, + 'background' => $this->urlGenerator->linkTo(Application::APP_ID, 'img/background/' . BackgroundService::DEFAULT_BACKGROUND_IMAGE), + default => '', + }; } /** @@ -166,8 +162,8 @@ public function getCacheFolder(): ISimpleFolder { * Get a file from AppData * * @param string $filename + * @return ISimpleFile * @throws NotFoundException - * @return \OCP\Files\SimpleFS\ISimpleFile * @throws NotPermittedException */ public function getCachedImage(string $filename): ISimpleFile { @@ -178,9 +174,6 @@ public function getCachedImage(string $filename): ISimpleFile { /** * Store a file for theming in AppData * - * @param string $filename - * @param string $data - * @return \OCP\Files\SimpleFS\ISimpleFile * @throws NotFoundException * @throws NotPermittedException */ @@ -342,10 +335,8 @@ public function cleanup() { /** * Check if Imagemagick is enabled and if SVG is supported * otherwise we can't render custom icons - * - * @return bool */ - public function shouldReplaceIcons() { + public function shouldReplaceIcons(): bool { $cache = $this->cacheFactory->createDistributed('theming-' . $this->urlGenerator->getBaseUrl()); if ($value = $cache->get('shouldReplaceIcons')) { return (bool)$value; diff --git a/apps/theming/lib/Themes/DarkHighContrastTheme.php b/apps/theming/lib/Themes/DarkHighContrastTheme.php index e6f1da94b4e8b..0c342fb8b1b5a 100644 --- a/apps/theming/lib/Themes/DarkHighContrastTheme.php +++ b/apps/theming/lib/Themes/DarkHighContrastTheme.php @@ -28,7 +28,6 @@ use OCA\Theming\ITheme; class DarkHighContrastTheme extends DarkTheme implements ITheme { - public function getId(): string { return 'dark-highcontrast'; } diff --git a/apps/theming/lib/Themes/DarkTheme.php b/apps/theming/lib/Themes/DarkTheme.php index 8d88de75834ed..e89af7387626e 100644 --- a/apps/theming/lib/Themes/DarkTheme.php +++ b/apps/theming/lib/Themes/DarkTheme.php @@ -28,7 +28,6 @@ use OCA\Theming\ITheme; class DarkTheme extends DefaultTheme implements ITheme { - public function getId(): string { return 'dark'; } diff --git a/apps/theming/lib/Themes/DefaultTheme.php b/apps/theming/lib/Themes/DefaultTheme.php index 68038f053e397..b428c0858a819 100644 --- a/apps/theming/lib/Themes/DefaultTheme.php +++ b/apps/theming/lib/Themes/DefaultTheme.php @@ -39,35 +39,19 @@ class DefaultTheme implements ITheme { use CommonThemeTrait; - public Util $util; - public ThemingDefaults $themingDefaults; - public IUserSession $userSession; - public IURLGenerator $urlGenerator; - public ImageManager $imageManager; - public IConfig $config; - public IL10N $l; - public IAppManager $appManager; - public string $defaultPrimaryColor; public string $primaryColor; - public function __construct(Util $util, - ThemingDefaults $themingDefaults, - IUserSession $userSession, - IURLGenerator $urlGenerator, - ImageManager $imageManager, - IConfig $config, - IL10N $l, - IAppManager $appManager) { - $this->util = $util; - $this->themingDefaults = $themingDefaults; - $this->userSession = $userSession; - $this->urlGenerator = $urlGenerator; - $this->imageManager = $imageManager; - $this->config = $config; - $this->l = $l; - $this->appManager = $appManager; - + public function __construct( + public Util $util, + public ThemingDefaults $themingDefaults, + public IUserSession $userSession, + public IURLGenerator $urlGenerator, + public ImageManager $imageManager, + public IConfig $config, + public IL10N $l, + public IAppManager $appManager, + ) { $this->defaultPrimaryColor = $this->themingDefaults->getDefaultColorPrimary(); $this->primaryColor = $this->themingDefaults->getColorPrimary(); diff --git a/apps/theming/lib/Themes/DyslexiaFont.php b/apps/theming/lib/Themes/DyslexiaFont.php index 3275a005c8d4a..f49654fcd05b8 100644 --- a/apps/theming/lib/Themes/DyslexiaFont.php +++ b/apps/theming/lib/Themes/DyslexiaFont.php @@ -28,7 +28,6 @@ use OCA\Theming\ITheme; class DyslexiaFont extends DefaultTheme implements ITheme { - public function getId(): string { return 'opendyslexic'; } @@ -77,7 +76,7 @@ public function getCustomCss(): string { url('$fontPathOtf') format('opentype'), url('$fontPathTtf') format('truetype'); } - + @font-face { font-family: 'OpenDyslexic'; font-style: normal; diff --git a/apps/theming/lib/Themes/HighContrastTheme.php b/apps/theming/lib/Themes/HighContrastTheme.php index e21e802fbe325..749428192a432 100644 --- a/apps/theming/lib/Themes/HighContrastTheme.php +++ b/apps/theming/lib/Themes/HighContrastTheme.php @@ -28,7 +28,6 @@ use OCA\Theming\ITheme; class HighContrastTheme extends DefaultTheme implements ITheme { - public function getId(): string { return 'light-highcontrast'; } diff --git a/apps/theming/lib/Themes/LightTheme.php b/apps/theming/lib/Themes/LightTheme.php index 7e6773992a10c..b806f108e7060 100644 --- a/apps/theming/lib/Themes/LightTheme.php +++ b/apps/theming/lib/Themes/LightTheme.php @@ -28,7 +28,6 @@ use OCA\Theming\ITheme; class LightTheme extends DefaultTheme implements ITheme { - public function getId(): string { return 'light'; } diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index 210029ae63694..48436a9c80871 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -54,17 +54,6 @@ use OCP\IUserSession; class ThemingDefaults extends \OC_Defaults { - - private IConfig $config; - private IL10N $l; - private ImageManager $imageManager; - private IUserSession $userSession; - private IURLGenerator $urlGenerator; - private ICacheFactory $cacheFactory; - private Util $util; - private IAppManager $appManager; - private INavigationManager $navigationManager; - private string $name; private string $title; private string $entity; @@ -78,38 +67,18 @@ class ThemingDefaults extends \OC_Defaults { private string $AndroidClientUrl; private string $FDroidClientUrl; - /** - * ThemingDefaults constructor. - * - * @param IConfig $config - * @param IL10N $l - * @param ImageManager $imageManager - * @param IUserSession $userSession - * @param IURLGenerator $urlGenerator - * @param ICacheFactory $cacheFactory - * @param Util $util - * @param IAppManager $appManager - */ - public function __construct(IConfig $config, - IL10N $l, - IUserSession $userSession, - IURLGenerator $urlGenerator, - ICacheFactory $cacheFactory, - Util $util, - ImageManager $imageManager, - IAppManager $appManager, - INavigationManager $navigationManager + public function __construct( + private IConfig $config, + private IL10N $l, + private IUserSession $userSession, + private IURLGenerator $urlGenerator, + private ICacheFactory $cacheFactory, + private Util $util, + private ImageManager $imageManager, + private IAppManager $appManager, + private INavigationManager $navigationManager, ) { parent::__construct(); - $this->config = $config; - $this->l = $l; - $this->imageManager = $imageManager; - $this->userSession = $userSession; - $this->urlGenerator = $urlGenerator; - $this->cacheFactory = $cacheFactory; - $this->util = $util; - $this->appManager = $appManager; - $this->navigationManager = $navigationManager; $this->name = parent::getName(); $this->title = parent::getTitle(); @@ -124,27 +93,27 @@ public function __construct(IConfig $config, $this->docBaseUrl = parent::getDocBaseUrl(); } - public function getName() { + public function getName(): string { return strip_tags($this->config->getAppValue('theming', 'name', $this->name)); } - public function getHTMLName() { + public function getHTMLName(): string { return $this->config->getAppValue('theming', 'name', $this->name); } - public function getTitle() { + public function getTitle(): string { return strip_tags($this->config->getAppValue('theming', 'name', $this->title)); } - public function getEntity() { + public function getEntity(): string { return strip_tags($this->config->getAppValue('theming', 'name', $this->entity)); } - public function getProductName() { + public function getProductName(): string { return strip_tags($this->config->getAppValue('theming', 'productName', $this->productName)); } - public function getBaseUrl() { + public function getBaseUrl(): string { return $this->config->getAppValue('theming', 'url', $this->url); } @@ -157,19 +126,19 @@ public function getSlogan(?string $lang = null) { return \OCP\Util::sanitizeHTML($this->config->getAppValue('theming', 'slogan', parent::getSlogan($lang))); } - public function getImprintUrl() { - return (string)$this->config->getAppValue('theming', 'imprintUrl', ''); + public function getImprintUrl(): string { + return $this->config->getAppValue('theming', 'imprintUrl', ''); } - public function getPrivacyUrl() { - return (string)$this->config->getAppValue('theming', 'privacyUrl', ''); + public function getPrivacyUrl(): string { + return $this->config->getAppValue('theming', 'privacyUrl', ''); } - public function getDocBaseUrl() { - return (string)$this->config->getAppValue('theming', 'docBaseUrl', $this->docBaseUrl); + public function getDocBaseUrl(): string { + return $this->config->getAppValue('theming', 'docBaseUrl', $this->docBaseUrl); } - public function getShortFooter() { + public function getShortFooter(): string { $slogan = $this->getSlogan(); $baseUrl = $this->getBaseUrl(); $entity = $this->getEntity(); @@ -188,11 +157,11 @@ public function getShortFooter() { $links = [ [ 'text' => $this->l->t('Legal notice'), - 'url' => (string)$this->getImprintUrl() + 'url' => $this->getImprintUrl() ], [ 'text' => $this->l->t('Privacy policy'), - 'url' => (string)$this->getPrivacyUrl() + 'url' => $this->getPrivacyUrl() ], ]; @@ -307,45 +276,31 @@ public function getLogo($useSvg = true): string { /** * Themed background image url - * - * @return string */ public function getBackground(): string { return $this->imageManager->getImageUrl('background'); } - /** - * @return string - */ - public function getiTunesAppId() { + public function getiTunesAppId(): string { return $this->config->getAppValue('theming', 'iTunesAppId', $this->iTunesAppId); } - /** - * @return string - */ - public function getiOSClientUrl() { + public function getiOSClientUrl(): string { return $this->config->getAppValue('theming', 'iOSClientUrl', $this->iOSClientUrl); } - /** - * @return string - */ - public function getAndroidClientUrl() { + public function getAndroidClientUrl(): string { return $this->config->getAppValue('theming', 'AndroidClientUrl', $this->AndroidClientUrl); } - /** - * @return string - */ - public function getFDroidClientUrl() { + public function getFDroidClientUrl(): string { return $this->config->getAppValue('theming', 'FDroidClientUrl', $this->FDroidClientUrl); } /** * @return array scss variables to overwrite */ - public function getScssVariables() { + public function getScssVariables(): array { $cacheBuster = $this->config->getAppValue('theming', 'cachebuster', '0'); $cache = $this->cacheFactory->createDistributed('theming-' . $cacheBuster . '-' . $this->urlGenerator->getBaseUrl()); if ($value = $cache->get('getScssVariables')) { @@ -391,9 +346,9 @@ public function getScssVariables() { * * @param string $app name of the app * @param string $image filename of the image - * @return bool|string false if image should not replaced, otherwise the location of the image + * @return bool|string false if image should not be replaced, otherwise the location of the image */ - public function replaceImagePath($app, $image) { + public function replaceImagePath($app, $image): bool|string { if ($app === '' || $app === 'files_sharing') { $app = 'core'; } @@ -502,19 +457,15 @@ public function undo($setting): string { /** * Color of text in the header and primary buttons - * - * @return string */ - public function getTextColorPrimary() { + public function getTextColorPrimary(): string { return $this->util->invertTextColor($this->getColorPrimary()) ? '#000000' : '#ffffff'; } /** * Color of text in the header and primary buttons - * - * @return string */ - public function getDefaultTextColorPrimary() { + public function getDefaultTextColorPrimary(): string { return $this->util->invertTextColor($this->getDefaultColorPrimary()) ? '#000000' : '#ffffff'; } diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php index a2f8ee0abad23..85f3bc63e9c3f 100644 --- a/apps/theming/lib/Util.php +++ b/apps/theming/lib/Util.php @@ -38,23 +38,17 @@ use OCP\IUserSession; class Util { - - private IConfig $config; - private IAppManager $appManager; - private IAppData $appData; - private ImageManager $imageManager; - - public function __construct(IConfig $config, IAppManager $appManager, IAppData $appData, ImageManager $imageManager) { - $this->config = $config; - $this->appManager = $appManager; - $this->appData = $appData; - $this->imageManager = $imageManager; + public function __construct( + private IConfig $config, + private IAppManager $appManager, + private IAppData $appData, + private ImageManager $imageManager, + ) { } /** * Should we invert the text on this background color? * @param string $color rgb color value - * @return bool */ public function invertTextColor(string $color): bool { return $this->colorContrast($color, '#ffffff') < 4.5; @@ -63,25 +57,17 @@ public function invertTextColor(string $color): bool { /** * Is this color too bright ? * @param string $color rgb color value - * @return bool */ public function isBrightColor(string $color): bool { - $l = $this->calculateLuma($color); - if ($l > 0.6) { - return true; - } else { - return false; - } + return $this->calculateLuma($color) > 0.6; } /** * get color for on-page elements: * theme color by default, grey if theme color is to bright * @param string $color - * @param ?bool $brightBackground - * @return string */ - public function elementColor($color, ?bool $brightBackground = null, ?string $backgroundColor = null, bool $highContrast = false) { + public function elementColor($color, ?bool $brightBackground = null, ?string $backgroundColor = null, bool $highContrast = false): string { if ($backgroundColor !== null) { $brightBackground = $brightBackground ?? $this->isBrightColor($backgroundColor); // Minimal amount that is possible to change the luminance @@ -140,10 +126,6 @@ public function darken(string $color, int $factor): string { * * Copied from cssphp, copyright Leaf Corcoran, licensed under MIT * - * @param int $red - * @param int $green - * @param int $blue - * * @return float[] */ public function toHSL(int $red, int $green, int $blue): array { @@ -210,7 +192,7 @@ public function hexToRGB(string $color): array { * @param $color * @return string base64 encoded radio button svg */ - public function generateRadioButton($color) { + public function generateRadioButton($color): string { $radioButtonIcon = '' . ''; return base64_encode($radioButtonIcon); @@ -221,7 +203,7 @@ public function generateRadioButton($color) { * @param string $app app name * @return string|ISimpleFile path to app icon / file of logo */ - public function getAppIcon($app) { + public function getAppIcon($app): ISimpleFile|string { $app = str_replace(['\0', '/', '\\', '..'], '', $app); try { $appPath = $this->appManager->getAppPath($app); @@ -237,7 +219,6 @@ public function getAppIcon($app) { } if ($this->config->getAppValue('theming', 'logoMime', '') !== '') { - $logoFile = null; try { $folder = $this->appData->getFolder('global/images'); return $folder->getFile('logo'); @@ -252,7 +233,7 @@ public function getAppIcon($app) { * @param string $image relative path to image in app folder * @return string|false absolute path to image */ - public function getAppImage($app, $image) { + public function getAppImage($app, $image): bool|string { $app = str_replace(['\0', '/', '\\', '..'], '', $app); $image = str_replace(['\0', '\\', '..'], '', $image); if ($app === "core") { @@ -297,32 +278,24 @@ public function getAppImage($app, $image) { * * @param string $svg content of a svg file * @param string $color color to match - * @return string */ - public function colorizeSvg($svg, $color) { - $svg = preg_replace('/#0082c9/i', $color, $svg); - return $svg; + public function colorizeSvg($svg, $color): string { + return preg_replace('/#0082c9/i', $color, $svg); } /** * Check if a custom theme is set in the server configuration - * - * @return bool */ - public function isAlreadyThemed() { - $theme = $this->config->getSystemValue('theme', ''); - if ($theme !== '') { - return true; - } - return false; + public function isAlreadyThemed(): bool { + return $this->config->getSystemValue('theme', '') !== ''; } - public function isBackgroundThemed() { + public function isBackgroundThemed(): bool { $backgroundLogo = $this->config->getAppValue('theming', 'backgroundMime', ''); return $backgroundLogo !== '' && $backgroundLogo !== 'backgroundColor'; } - public function isLogoThemed() { + public function isLogoThemed(): bool { return $this->imageManager->hasImage('logo') || $this->imageManager->hasImage('logoheader'); }