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.

Sign up for GitHub

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

Feature/pfe 1828 update radio checkmarks dg #1017

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Feature/pfe 1828 update radio checkmarks dg #1017

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
28a1b40
pfe-1828 Updated radiobutton hover, checkmarks hover, color, bg color
PriJoh Nov 8, 2024
f1e6955
pfe-1828 Added circle svg. Changed color, bg color and border on hove…
PriJoh Nov 12, 2024
598a072
pfe-1828 Removed blank spaces
PriJoh Nov 13, 2024
dc09130
pfe-1828 Changed border to outline and removed redundant code
PriJoh Nov 26, 2024
b5c4e53
pfe-1828 Nested disabled circle.svg up to previous rule
PriJoh Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/icons/payex/circlepx.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions src/icons/payex/circlepx_disabled.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
108 changes: 108 additions & 0 deletions src/less/components/payex/radio.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,117 @@

input[type="radio"] {
+ label {

&:before {
border: 2px solid @radiobutton-border;
}
}

&:is(:hover, :focus-visible) + label:before {
box-shadow: 0 0 0 0.375rem var(--gray);
}

&:checked + label:before {
background-position: center;
background-image: url("../../../icons/payex/circlepx.svg");
}
}
}

.radio input[type=radio][disabled]:checked+label:before {
background-image: url("../../../icons/payex/circlepx_disabled.svg");
}
Copy link
Collaborator

@goldenraphti goldenraphti Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

you can nest it inside the previous rule

Suggested change
&:is(:hover, :focus-visible) + label:before {
box-shadow: 0 0 0 0.375rem var(--gray);
}
&:checked + label:before {
background-position: center;
background-image: url("../../../icons/payex/circlepx.svg");
}
}
}
.radio input[type=radio][disabled]:checked+label:before {
background-image: url("../../../icons/payex/circlepx_disabled.svg");
}
&:is(:hover, :focus-visible) + label:before {
box-shadow: 0 0 0 0.375rem var(--gray);
}
&:checked + label:before {
background-position: center;
background-image: url("../../../icons/payex/circlepx.svg");
}
&[disabled]:checked+label:before {
background-image: url("../../../icons/payex/circlepx_disabled.svg");
}
}
}


.checkmark {
Copy link
Collaborator

Choose a reason for hiding this comment

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

try adding .radio to the selector, this way you're adding teh same specificity than in the "base" css file for radio component, and therefore it might likely decrease a lot the need for using !important in your declarations

So either by adding the .radio selector before .checkmark ( = .radio.checkmark ) OR by nesting the .checkmark { ... } rule inside the over-arching .radio { ... } parent


label {
border: 1px solid var(--gray);
outline: none !important;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why removing outline to replace it by borders ?
If the change is motivated by another reason than the change of color please let meknow, I'm very open to revisit how we style this component, especially if it's because you've spotted some accessibility issues or something like this.

If the change is only to achieve a change of color then I think you can achieve if more easily by only modifying the outline color:

Suggested change
border: 1px solid var(--gray);
outline: none !important;
outline-color: var(--gray);

this goes for teh rest of the file, everywhere outline is changed for border

color: var(--light-black);

.radio.checkmark label .checkmark-icon {
background-color: var(--brand-secondary);
}

&:hover {
border: 1px solid var(--gray) !important;
Copy link
Collaborator

Choose a reason for hiding this comment

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

if only the color is modified on hover, then use a *-color property

e.g.

border-color: var(--gray);

OR if you've switched back to outline:

outline-color: var(--gray);

BUT beyond that:
do we need this declaration?
the [outline/border]-color value is already declared asvar(--gray), so I do not see the reason why adding it.

background-color: var(--light-gray) !important;
color: var(--brand-secondary) !important;
Comment on lines +42 to +43
Copy link
Collaborator

Choose a reason for hiding this comment

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

!important is not needed if you add .radio to the .checkmark selector

Suggested change
background-color: var(--light-gray) !important;
color: var(--brand-secondary) !important;
background-color: var(--light-gray);
color: var(--brand-secondary);


.checkmark-icon,
> i {
background-color: var(--brand-secondary) !important;
}

.checkmark-icon::before, .checkmark label:hover > i::before {
border-color: var(--light-gray) !important;
}

.checkmark-icon::before,
.checkmark label:hover > i::before {
border-color: var(--light-gray) !important;
}
}
}
}

.radio.checkmark input[type="radio"]:checked + label:hover {
border: 1px solid var(--brand-secondary);
outline: none !important;
}

.radio.checkmark input[type="radio"]:checked + label {
border: 1px solid var(--brand-secondary);
outline: 3px solid var(--brand-secondary) !important;
}

.radio.checkmark input[type="radio"]:disabled:checked + label {
color: var(--white);
border: 1px solid var(--gray) !important;
outline: none !important;

.checkmark-icon,
> i {
background-color: var(--white);
}
.checkmark-icon::before, .checkmark label:disabled > i::before {
border-color: var(--gray) !important;
}
}

.radio.checkmark input[type="radio"]:disabled + label:hover {
color: var(--white) !important;
border: 1px solid var(--gray) !important;
outline: none !important;
background-color: var(--gray) !important;
cursor: not-allowed !important;

.checkmark-icon,
> i {
background-color: var(--white) !important;
}
.checkmark-icon::before, .checkmark label:disabled > i::before {
border-color: var(--gray) !important;
}
}

.radio.checkmark input[type="radio"]:disabled:checked:hover + label {
color: var(--white);
border: 1px solid var(--gray) !important;
outline: none !important;

.checkmark-icon,
> i {
background-color: var(--white);
}
.checkmark-icon::before, .checkmark label:disabled > i::before {
border-color: var(--gray) !important;
}
}

.radio.checkmark input[type="radio"]:disabled + label {
color: var(--white);
border: 1px solid var(--gray) !important;
outline: none !important;
background-color: var(--gray) !important;
}
1 change: 1 addition & 0 deletions src/less/variables-payex.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ body {
:root {
/* Grayscale */
--black: #000000;
--light-black: #2F2424;
--list-gray: #999999;
--light-gray: #f4f4f4;
--gray: #c8c8c8;
Expand Down
Loading