Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Checkbox][WIP] New features#2010

Draft
prudho wants to merge 3 commits intofomantic:developfrom
prudho:enhanced-checkboxes
Draft

[Checkbox][WIP] New features#2010
prudho wants to merge 3 commits intofomantic:developfrom
prudho:enhanced-checkboxes

Conversation

Copy link
Contributor

prudho commented Jul 7, 2021

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 and outline checkboxes too see what it's possible to implement.

The others options i'm thinking are:

  • icons instead of check mark.
  • animated checks (js and Transition are paving the way).
  • label text depending on checkbox state...

What do you guys think about this ?

Testcase

JSFiddle

Screenshot

Related issues

#393, #550, #2009

azmeuk, teacher-zhou, and allweDepth reacted with heart emoji
prudho added type/feat Any feature requests or improvements lang/css Anything involving CSS lang/javascript Anything involving JavaScript type/discussion Anything which is up for discussion state/awaiting-reviews Pull requests which are waiting for reviews state/awaiting-docs Pull requests which need doc changes/additions state/awaiting-changes Anything which is awaiting changes tag/help-wanted Issues which need help to fix or implement labels Jul 7, 2021
prudho requested review from ColinFrick, exoego, ko2in, lubber-de and y0hami July 7, 2021 14:50
prudho added this to the 2.9.0 milestone Jul 7, 2021
prudho marked this pull request as draft July 7, 2021 14:52
Copy link
Member

lubber-de commented Jul 7, 2021

I like the approach to make outline optional !
I also like your additional ideas

lubber-de requested changes Aug 7, 2021
Copy link
Member

lubber-de left a 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.

Comment on lines +813 to +816
.ui.@{color}.toggle.checkbox input:checked ~ label::before,
.ui.@{color}.slider.checkbox input:checked ~ label::before {
background-color: @color !important;
}
Copy link
Member

lubber-de Aug 7, 2021

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!)

Suggested change
.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;
}

Copy link
Contributor Author

prudho Aug 9, 2021

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...

Copy link
Member

lubber-de Aug 9, 2021

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

Copy link
Member

lubber-de Aug 9, 2021

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

Copy link
Contributor Author

prudho commented Aug 9, 2021

Icon instead of checkmark is doable:

But it will require more tests and code than previous modifications. Should I create another PR ?

lubber-de reacted with thumbs up emoji

Copy link
Member

lubber-de commented Aug 9, 2021

But it will require more tests and code than previous modifications. Should I create another PR ?

Up to you Just make sure that each PR is based on current develop (or do everything in one PR)

Copy link
Member

lubber-de commented Aug 9, 2021 *
edited
Loading

I have a suggestion/idea/question:
What about replacing outline by bordered?
We already have bordered icon and bordered image

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?

Copy link
Contributor Author

prudho commented Aug 10, 2021

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).

Copy link
Contributor Author

prudho commented Aug 10, 2021

I've added the icon variation. Works well overall, except for sizes, i'm not comfortable with advanced scss yet

lubber-de requested changes Aug 12, 2021
/* Checked */
.ui.checkbox input:checked ~ .box:after,
.ui.checkbox input:checked ~ label:after {
.ui.checkbox:not(.icon) input:checked ~ .box:after,
Copy link
Member

lubber-de Aug 10, 2021

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

@checkboxCheckTop: 0;
@checkboxCheckLeft: 0;
@checkboxCheckSize: @checkboxSize;
@checkboxIconCheckFontSize: 12px;
Copy link
Member

lubber-de Aug 10, 2021

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 {
Copy link
Member

lubber-de Aug 10, 2021 *
edited
Loading

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....)

lubber-de modified the milestones: 2.9.0, 2.9.x Dec 27, 2021
lubber-de mentioned this pull request Feb 4, 2022
lubber-de removed the state/awaiting-reviews Pull requests which are waiting for reviews label Feb 5, 2022
lubber-de modified the milestones: 2.9.x, 2.10.x Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lubber-de lubber-de requested changes

exoego Awaiting requested review from exoego

ko2in Awaiting requested review from ko2in

ColinFrick Awaiting requested review from ColinFrick

y0hami Awaiting requested review from y0hami

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

lang/css Anything involving CSS lang/javascript Anything involving JavaScript state/awaiting-changes Anything which is awaiting changes state/awaiting-docs Pull requests which need doc changes/additions tag/help-wanted Issues which need help to fix or implement type/discussion Anything which is up for discussion type/feat Any feature requests or improvements

Projects

None yet

Milestone

2.10.x

Development

Successfully merging this pull request may close these issues.

2 participants