-
Notifications
You must be signed in to change notification settings - Fork 13.3k
feat(config): add icon options and align ionic theme with md defaults #31418
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
base: next
Are you sure you want to change the base?
Changes from all commits
7057225
69200e8
4023d2a
d5c3159
662509c
ccf41e1
9882ad6
a675914
55b217c
403df5d
c591b44
57cadbf
550e889
861c01c
fddf4eb
393a0bc
8b271d6
73ce038
badfb51
d513494
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
|
Member
Author
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. The default icons have changed for the |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| /** | ||
| * @prop --size: Size of the checkbox icon | ||
| * | ||
| * @prop --checkbox-background: Background of the checkbox icon | ||
| * @prop --checkbox-background-checked: Background of the checkbox icon when checked | ||
| * | ||
| * @prop --border-color: Border color of the checkbox icon | ||
|
|
@@ -97,17 +98,52 @@ input { | |
| } | ||
|
|
||
| .native-wrapper { | ||
| @include mixins.border-radius(var(--border-radius)); | ||
|
|
||
| display: flex; | ||
|
|
||
| position: relative; | ||
|
|
||
| flex-shrink: 0; | ||
|
|
||
| align-items: center; | ||
|
|
||
| width: var(--size); | ||
| height: var(--size); | ||
|
|
||
| transition: var(--transition); | ||
|
|
||
| border-width: var(--border-width); | ||
| border-style: var(--border-style); | ||
| border-color: var(--border-color); | ||
|
|
||
| background: var(--checkbox-background); | ||
|
|
||
| box-sizing: border-box; | ||
| } | ||
|
Comment on lines
100
to
123
Member
Author
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. These styles were the same in the |
||
|
|
||
| // Checkbox Icon | ||
| // ---------------------------------------------------------------- | ||
|
|
||
| /** | ||
| * Apply the color to the ion-icon container so that the default | ||
| * checkmark icon and any configured icon apply color consistently. | ||
| */ | ||
| .checkbox-icon { | ||
| position: relative; | ||
| color: var(--checkmark-color); | ||
|
|
||
| opacity: 0; | ||
| } | ||
|
|
||
| .checkbox-icon svg { | ||
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .checkbox-icon path { | ||
| opacity: 0; | ||
| fill: none; | ||
| stroke: currentColor; | ||
| stroke-width: var(--checkmark-width); | ||
|
Member
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. This fill to stroke change is what makes the Probably a follow-up rather than something for this PR, but back-button, breadcrumb, fab-button and a few of the other components whose
Member
Author
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. Added: 8b271d6 I think getting all of the themes tested properly should be part of the modular ionic work, personally. Ideally it should be done at the time of migrating each component. |
||
| } | ||
|
|
||
| // Checkbox Bottom Content | ||
|
|
@@ -247,15 +283,15 @@ input { | |
| // Checked / Indeterminate Checkbox | ||
| // --------------------------------------------- | ||
|
|
||
| :host(.checkbox-checked) .checkbox-icon, | ||
| :host(.checkbox-indeterminate) .checkbox-icon { | ||
| :host(.checkbox-checked) .native-wrapper, | ||
| :host(.checkbox-indeterminate) .native-wrapper { | ||
| border-color: var(--border-color-checked); | ||
|
|
||
| background: var(--checkbox-background-checked); | ||
| } | ||
|
|
||
| :host(.checkbox-checked) .checkbox-icon path, | ||
| :host(.checkbox-indeterminate) .checkbox-icon path { | ||
| :host(.checkbox-checked) .checkbox-icon, | ||
| :host(.checkbox-indeterminate) .checkbox-icon { | ||
| opacity: 1; | ||
| } | ||
|
|
||
|
|
||
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.
The
@propdoc for--checkbox-backgroundprobably wants to move into the common stylesheet alongside it, otherwise this file keeps listing it for ios and md only. The split predates the PR, but you already moved--checkmark-width's doc across as part of the same consolidation. Up to you!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.
57cadbf