-
-
Notifications
You must be signed in to change notification settings - Fork 333
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
[Checkbox][WIP] New features #2010
base: develop
Are you sure you want to change the base?
Conversation
I like the approach to make |
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.
I know, this PR is marked as Draft and WIP, but i wanted to give "official" Feedback beside my other comment anyway 🙂 for the current code.
.ui.@{color}.toggle.checkbox input:checked ~ label::before, | ||
.ui.@{color}.slider.checkbox input:checked ~ label::before { | ||
background-color: @color !important; | ||
} |
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.
Beside the variable name: Any chance to increase specificity without using !important
? (Haven't tested myself yet!)
.ui.@{color}.toggle.checkbox input:checked ~ label::before, | |
.ui.@{color}.slider.checkbox input:checked ~ label::before { | |
background-color: @color !important; | |
} | |
.ui.ui.@{color}.toggle.checkbox input:checked ~ label::before, | |
.ui.ui.@{color}.slider.checkbox input:checked ~ label::before { | |
background-color: @c; | |
} |
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.
I tried .ui.ui
and .ui.ui.ui
, but still need that !important
to work... 😞
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.
It's because there is an existing !important
at
.ui.toggle.checkbox input:checked ~ label:before {
background-color: #2185D0 !important;
}
We would need to remove the !important
from there too
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.
If you check the checkbox.less there are still a few !important
for toggle and slider which are causing this. This PR is a good chance to get rid of them as well 😉
Up to you 😉 Just make sure that each PR is based on current develop (or do everything in one PR) |
I have a suggestion/idea/question: https://fomantic-ui.com/elements/icon.html#bordered-colored However, a checkbox/radio always has an "outline" or is "bordered". Your new feature does only make the outline/border more prominent/bold/colored Opinion? |
Yeah I don't know how to really name it, since I "copy" class name from another framework. And I struck to find another class name for a no bordered checkbox as well (well the border disapear when checked). |
I've added the |
@@ -16,8 +16,8 @@ | |||
} | |||
|
|||
/* Checked */ | |||
.ui.checkbox input:checked ~ .box:after, | |||
.ui.checkbox input:checked ~ label:after { | |||
.ui.checkbox:not(.icon) input:checked ~ .box:after, |
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.
As we removed the .box
selectors by #2049 you may merge the current develop into this PR and remove the .box afterwwards
@@ -26,6 +26,7 @@ | |||
@checkboxCheckTop: 0; | |||
@checkboxCheckLeft: 0; | |||
@checkboxCheckSize: @checkboxSize; | |||
@checkboxIconCheckFontSize: 12px; |
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.
We have the @{size}CheckboxSize
variables already, you may reuse them (i didn't test 🙄 )
Also, please use em
instead of px
line-height: @checkboxLineHeight; | ||
} | ||
|
||
.ui.icon.checkbox input:not(:checked) ~ label i { |
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.
That means, that any icon inside the label (and not supposed to be used for the "check" icon) will vanish.
If we keep this logic, this would mean to define that icon.checkbox
variations do not support Icons in the label itself
Just a thought (I don't request the following):
What about using the icon font family on icon.checkbox
instead, so dont have to deal with a separate i
tag? That would probably also fix the sizing issues and is the same approach as the current check icon.
Infact, this approach would mean to adjust all icons selectors which would be quite a lot.
A long time back we discussed data attributes "data-icon" which would be independent of the tag and could be reused here (yes, that would be a separate PR and huge breaking change....)
Description
Following #2009, I think that indeed checkboxes need some fancy new features too. So I open this PR as a draft and discussion topic.
For now, I just add
colored
andoutline
checkboxes too see what it's possible to implement.The others options i'm thinking are:
Transition
are paving the way).What do you guys think about this ?
Testcase
JSFiddle
Screenshot
Related issues
#393, #550, #2009