-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
add descriptions for background pictures #41216
Changes from all commits
18b1247
7dc91c4
513efdc
2a974aa
773fe16
a6cf2ea
95dac5b
810dd5e
b45aa3a
46e7941
732dbbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,6 +46,7 @@ class DefaultTheme implements ITheme { | |
public IConfig $config; | ||
public IL10N $l; | ||
public IAppManager $appManager; | ||
public BackgroundService $backgroundService; | ||
|
||
public string $defaultPrimaryColor; | ||
public string $primaryColor; | ||
|
@@ -57,7 +58,8 @@ public function __construct(Util $util, | |
ImageManager $imageManager, | ||
IConfig $config, | ||
IL10N $l, | ||
IAppManager $appManager) { | ||
IAppManager $appManager, | ||
BackgroundService $backgroundService) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this is now breaking many tests (34 afaics). @ChristophWurst Any other idea how I can do this without having to refactor everything? Adding backgroundservice to this constructor at least does not seem to work... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have any other ideas. Can you adjust/fix the tests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if I would move the commonthemetrait? Do you think this would make the logic easier? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Try There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets see #41235 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could I ask you to try your changes locally? Using CI for trial and error is really expensive and slows down real builds. |
||
$this->util = $util; | ||
$this->themingDefaults = $themingDefaults; | ||
$this->userSession = $userSession; | ||
|
@@ -66,6 +68,7 @@ public function __construct(Util $util, | |
$this->config = $config; | ||
$this->l = $l; | ||
$this->appManager = $appManager; | ||
$this->backgroundService = $backgroundService; | ||
|
||
$this->defaultPrimaryColor = $this->themingDefaults->getDefaultColorPrimary(); | ||
$this->primaryColor = $this->themingDefaults->getColorPrimary(); | ||
|
@@ -210,7 +213,7 @@ public function getCSSVariables(): array { | |
// Primary variables | ||
$variables = array_merge($variables, $this->generatePrimaryVariables($colorMainBackground, $colorMainText)); | ||
$variables = array_merge($variables, $this->generateGlobalBackgroundVariables()); | ||
$variables = array_merge($variables, $this->generateUserBackgroundVariables()); | ||
$variables = array_merge($variables, $this->generateUserBackgroundVariables($this->backgroundService)); | ||
|
||
return $variables; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reviewers: Please review each description carefully. Does the text look fine?