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

remove slight transparency of primary action button, ref #1615 #1903

Merged
merged 2 commits into from
Oct 31, 2016

Conversation

jancborchardt
Copy link
Member

Fix the first part of @ChristophWurst’s comment in #1615 (comment) about the log in button when themed

@juliushaertl how would I use the class .icon-confirm (instead of .icon-confirm-white) when the primary color is set so the text switches to black? :)

cc @nextcloud/designers

@jancborchardt jancborchardt added bug design Design, UI, UX, etc. 2. developing Work in progress feature: theming labels Oct 25, 2016
@jancborchardt jancborchardt added this to the Nextcloud 11.0 milestone Oct 25, 2016
@mention-bot
Copy link

@jancborchardt, thanks for your PR! By analyzing the history of the files in this pull request, we identified @juliushaertl, @LukasReschke and @schiessle to be potential reviewers.

@juliusknorr
Copy link
Member

juliusknorr commented Oct 25, 2016

@jancborchardt The simple way would be to add a custom css rule over here and overwrite the icon used in the button:

if($this->util->invertTextColor($color)) {
$responseCss .= '#header .header-appname, #expandDisplayName { color: #000000; }' . "\n";
$responseCss .= '#header .icon-caret { background-image: url(\'' . \OC::$WEBROOT . '/core/img/actions/caret-dark.svg\'); }' . "\n";
$responseCss .= '.searchbox input[type="search"] { background: transparent url(\'' . \OC::$WEBROOT . '/core/img/actions/search.svg\') no-repeat 6px center; color: #000; }' . "\n";
$responseCss .= '.searchbox input[type="search"]:focus,.searchbox input[type="search"]:active,.searchbox input[type="search"]:valid { color: #000; border: 1px solid rgba(0, 0, 0, .5); }' . "\n";
$responseCss .= '.nc-theming-contrast {color: #000000}' . "\n";
$responseCss .= '.ui-widget-header { color: #000000; }' . "\n";
} else {

And of course adapt the unit tests to match the changes. 😉

@jancborchardt
Copy link
Member Author

@juliushaertl thanks a lot! Fixed it and hopefully I also fixed the tests correctly. :D

Please review @nextcloud/designers

@jancborchardt jancborchardt added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 25, 2016
@codecov-io
Copy link

Current coverage is 57.74% (diff: 100%)

Merging #1903 into master will increase coverage by 0.25%

@@             master      #1903   diff @@
==========================================
  Files          1075       1078      +3   
  Lines         61215      62411   +1196   
  Methods        6864       7075    +211   
  Messages          0          0           
  Branches          0          0           
==========================================
+ Hits          35194      36039    +845   
- Misses        26021      26372    +351   
  Partials          0          0           

Sunburst

Diff Coverage File Path
•••••••••• 100% apps/theming/lib/Controller/ThemingController.php

Powered by Codecov. Last update 324e5b0...cbd9fb3

@jancborchardt
Copy link
Member Author

@LukasReschke @MorrisJobke can we deactivate the codecov comments? It’s already shown in the PR summary below and is a lot of noise. :)

@MorrisJobke
Copy link
Member

@LukasReschke @MorrisJobke can we deactivate the codecov comments? It’s already shown in the PR summary below and is a lot of noise. :)

#1949

@juliusknorr
Copy link
Member

👍

@LukasReschke
Copy link
Member

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug design Design, UI, UX, etc. feature: theming
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants