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

Transparent white checkbox #3341

Merged
merged 1 commit into from
Feb 14, 2017
Merged

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Jan 31, 2017

Following #3257 (comment)
More-or-less related: #3257

@nextcloud/designers what do you think? We give up css colouring but we gain transparency, is it worth it?

Technology used: we now have a big white square with a transparent mark on it (checked or mixed) and we set is as background with an hidden overflow .

capture d ecran_2017-01-31_19-30-12

Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
@skjnldsv skjnldsv added this to the Nextcloud 12.0 milestone Jan 31, 2017
@skjnldsv skjnldsv self-assigned this Jan 31, 2017
@mention-bot
Copy link

@skjnldsv, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jancborchardt to be a potential reviewer.

@codecov-io
Copy link

Codecov Report

Merging #3341 into master will not impact coverage.

@@            Coverage Diff            @@
##             master    #3341   +/-   ##
=========================================
  Coverage     53.98%   53.98%           
=========================================
  Files          1303     1303           
  Lines         80377    80377           
  Branches       1253     1253           
=========================================
  Hits          43394    43394           
  Misses        36983    36983

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db5af89...8d5bb0b. Read the comment docs.

@ChristophWurst
Copy link
Member

If I check out this branch I do not see any difference to master. Did you include all your changes?

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 6, 2017

Because the difference is barely noticeable.
I don't even have an example currently on nextcloud where this is used X)

You can add a dark background and set the input class to checkbox--white if you want to test.

@MariusBluem
Copy link
Member

You can add a dark background and set the input class to checkbox--white if you want to test.

Hope this will help your implementation of the dark mode, I am waiting for from an accessibility point of view 😉 #1692

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 6, 2017

@MariusBluem When you think all this scss mess was created from this dark mode pr... 😂
How far have we got now! What I'm waiting the most is the variables. It should be a game changer for the dark mode!

@jancborchardt
Copy link
Member

The only real difference is on the log in screen. @skjnldsv do you think it is worth the effort to fix that or better go with pure CSS?

Because we need to definitely fix #3257

@skjnldsv
Copy link
Member Author

@jancborchardt since the white checkbox is supposed to be white I don't see an issue there.
It shouldn't be too much trouble for someone to customise since it's a white only contrast. The other checkbox is entirely different (colour of border, shadow, checkmark...) :)

@jancborchardt
Copy link
Member

Ok, I will be able to test later today :)

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Seems to work nicely 👍

@jancborchardt jancborchardt merged commit 9f74858 into master Feb 14, 2017
@jancborchardt jancborchardt deleted the transparent-white-checkbox branch February 14, 2017 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants