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

feat(css): use native selectors #2050

Merged
merged 8 commits into from
Jun 3, 2024
Merged

feat(css): use native selectors #2050

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
321f5c6
refactor(checkbox): ✅ Use native selectors
poi33 May 20, 2024
3b998a6
refactor(fieldset): 🪧 Use native selectors
poi33 May 21, 2024
6eee9d2
refactor(NativeSelect): 🪧 Use native selectors
poi33 May 21, 2024
4ca1660
refactor(textarea): 🪧 Use native selectors
poi33 May 22, 2024
b7c0409
refactor(textfield): 🪧 use native selectors
poi33 May 22, 2024
d20e64f
refactor(radio): 🪧 Use native selectors
poi33 May 22, 2024
b5c9fec
refactor(switch): 🪧 Use native selectors
poi33 May 22, 2024
b7367c0
refactor(search): 🪧 Use native selectors
poi33 May 22, 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
34 changes: 17 additions & 17 deletions packages/css/checkbox.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,24 +59,18 @@
grid-column: 2;
}

.fds-checkbox--disabled > .fds-checkbox__input,
.fds-checkbox--disabled > .fds-checkbox__label,
.fds-checkbox--disabled > .fds-checkbox__input::before {
cursor: not-allowed;
}

.fds-checkbox--disabled > .fds-checkbox__label,
.fds-checkbox--disabled > .fds-checkbox__input,
.fds-checkbox--disabled > .fds-checkbox__description {
opacity: var(--fds-opacity-disabled);
}

.fds-checkbox--readonly > .fds-checkbox__label,
.fds-checkbox--readonly > .fds-checkbox__input,
.fds-checkbox--readonly > .fds-checkbox__input::before {
cursor: default;
}

.fds-checkbox__input:disabled,
.fds-checkbox__input:disabled ~ .fds-checkbox__label,
.fds-checkbox__input:disabled::before {
cursor: not-allowed;
}

.fds-checkbox__input:focus-visible {
outline-offset: 0;
outline: var(--fds-checkbox-focus-border-width) solid var(--fds-semantic-border-focus-outline);
Expand All @@ -103,6 +97,12 @@
--fds-checkbox-background: var(--fds-semantic-surface-neutral-subtle);
}

.fds-checkbox__input:disabled,
.fds-checkbox__input:disabled ~ .fds-checkbox__label,
.fds-checkbox__input:disabled ~ .fds-checkbox__description {
opacity: var(--fds-opacity-disabled);
}

.fds-checkbox__input:checked:not(:focus-visible) {
box-shadow: inset 0 0 0 2px var(--fds-checkbox-border-color);
}
Expand All @@ -125,28 +125,28 @@
/* Only use hover for non-touch devices to prevent sticky-hovering
"input:not(:read-only)" does not work so using ".container:not(.readonly) >" instead */
@media (hover: hover) and (pointer: fine) {
.fds-checkbox:not(.fds-checkbox--readonly, .fds-checkbox--disabled) > .fds-checkbox__label:hover,
.fds-checkbox:not(.fds-checkbox--readonly, .fds-checkbox--disabled) > .fds-checkbox__input:hover + .fds-checkbox__label {
.fds-checkbox:not(.fds-checkbox--readonly) .fds-checkbox__input:not(:disabled) ~ .fds-checkbox__label:hover,
.fds-checkbox:not(.fds-checkbox--readonly) .fds-checkbox__input:hover:not(:disabled) ~ .fds-checkbox__label {
color: var(--fds-semantic-text-action-hover);
}

.fds-checkbox:not(.fds-checkbox--readonly, .fds-checkbox--disabled) > .fds-checkbox__input:hover:not(:checked) {
.fds-checkbox:not(.fds-checkbox--readonly) .fds-checkbox__input:hover:not(:checked, :disabled) {
--fds-checkbox-border-color: var(--fds-semantic-border-input-hover);

box-shadow:
var(--fds-checkbox-border__hover),
inset 0 0 0 2px var(--fds-checkbox-border-color);
}
Comment on lines 125 to 139
Copy link
Member

@Barsnes Barsnes May 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Could we solve readonly by checking .fds-checkbox:not([readonly]), or add aria-readonly="true/false"?
I think it would be nice if we can eliminate the .fds-checkbox--readonly class, and only use data-attributes or selectors.

If that works we can do it for the other components as well 💪🏻

Copy link
Collaborator Author

@poi33 poi33 May 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Could we solve readonly by checking .fds-checkbox:not([readonly]),

I was thinking the same thing! 💡 This would make it so much easier to work with, but Readonly is only supported by input types that can change/mutate its value. Checkboxes apparently have two fixed values check or not.

or add aria-readonly="true/false"?

It is used in some places already (also in css). I was thinking about using the aria attr too glad you brought it up.
The downside of this is that the classes might be a little more complex to understand, and its a bit more work to add everywhere.

Nice suggestion, think I'll start with adding aria-readonly attributes and connect the css to those attributes. ✏️

Copy link
Collaborator

@mimarz mimarz May 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Lets try these out :)

I suggest however we also do some research on whats the best practice here. Attr selectors also have worse performance than pure classes, but its not exactly big theses days.

I am a bit worried about the longterm development and maintenance for using attr selectors as we are breaking seperation of concern with using attr selectors as we making a hard-link between styling and html. There might be edge-cases where this is might be a problem and also limits composability of the css.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Readonly-state is a tough one because as mentioned sometimes is enable by readonly, other times aria-readonly depending on the html element. 🤔

Did a quick search on support for using aria-readonly and SR, might be a bit outdated by its things to test for: https://adrianroselli.com/2022/11/brief-note-on-aria-readonly-support-html.html

Copy link
Member

Choose a reason for hiding this comment

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

We could always do two selectors for a style 🤔

.fds-checkbox:not([readonly]),
.fds-checkbox:not([aria-readonly]) {
...
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest just keeping it as a readonly class for now, we can always change it later if we find out thats a better solution. I am however not sure its the best approach so lets save it for #2065.


.fds-checkbox:not(.fds-checkbox--readonly, .fds-checkbox--disabled) > .fds-checkbox__input:hover:checked {
.fds-checkbox:not(.fds-checkbox--readonly) .fds-checkbox__input:hover:checked:not(:disabled) {
--fds-checkbox-border-color: var(--fds-semantic-border-input-hover);

box-shadow:
var(--fds-checkbox-border__hover),
inset 0 0 0 2px var(--fds-checkbox-border-color);
}

.fds-checkbox:not(.fds-checkbox--readonly, .fds-checkbox--disabled) > .fds-checkbox__input:hover:checked:focus-visible {
.fds-checkbox:not(.fds-checkbox--readonly) .fds-checkbox__input:hover:checked:focus-visible:not(:disabled) {
box-shadow:
var(--fds-checkbox-border__hover),
inset 0 0 0 var(--fds-checkbox-focus-border-width) var(--fds-semantic-border-focus-boxshadow);
Expand Down
4 changes: 2 additions & 2 deletions packages/css/fieldset.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
width: 1.2em;
}

.fds-fieldset--disabled .fds-fieldset__legend,
.fds-fieldset--disabled .fds-fieldset__description {
.fds-fieldset:disabled .fds-fieldset__legend,
.fds-fieldset:disabled .fds-fieldset__description {
color: var(--fds-semantic-border-neutral-subtle);
}

Expand Down
6 changes: 3 additions & 3 deletions packages/css/native-select.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@
gap: var(--fds-spacing-2);
}

.fds-native-select--disabled {
.fds-native-select--container:has(.fds-native-select:disabled) {
opacity: var(--fds-opacity-disabled);
}

.fds-native-select--disabled .fds-native-select {
.fds-native-select:disabled {
cursor: not-allowed;
}

Expand All @@ -63,7 +63,7 @@
border-color: var(--fds-semantic-border-neutral-default);
}

.fds-native-select--error > .fds-native-select:not(:focus-visible) {
.fds-native-select--error>.fds-native-select:not(:focus-visible) {
border-color: var(--fds-semantic-border-danger-default);
box-shadow: inset 0 0 0 1px var(--fds-semantic-border-danger-default);
}
Expand Down
31 changes: 15 additions & 16 deletions packages/css/radio.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,22 +59,21 @@
grid-column: 2;
}

.fds-radio--disabled > .fds-radio__input,
.fds-radio--disabled > .fds-radio__label,
.fds-radio--disabled > .fds-radio__input::before {
cursor: not-allowed;
.fds-radio--readonly > .fds-radio__input,
.fds-radio--readonly > .fds-radio__label,
.fds-radio--readonly > .fds-radio__input::before {
cursor: default;
}

.fds-radio--disabled > .fds-radio__label,
.fds-radio--disabled > .fds-radio__input,
.fds-radio--disabled > .fds-radio__description {
.fds-radio:has(.fds-radio__input:disabled) > .fds-radio__description {
opacity: var(--fds-opacity-disabled);
}

.fds-radio--readonly > .fds-radio__input,
.fds-radio--readonly > .fds-radio__label,
.fds-radio--readonly > .fds-radio__input::before {
cursor: default;
.fds-radio__input:disabled,
.fds-radio__input:disabled::before,
.fds-radio:has(.fds-radio__input:disabled) > .fds-radio__label {
cursor: not-allowed;
opacity: var(--fds-opacity-disabled);
}

.fds-radio__input:focus-visible {
Expand Down Expand Up @@ -113,20 +112,20 @@
/* Only use hover for non-touch devices to prevent sticky-hovering
"input:not(:read-only)" does not work so using ".container:not(.readonly) >" instead */
@media (hover: hover) and (pointer: fine) {
.fds-radio:not(.fds-radio--readonly, .fds-radio--disabled) > .fds-radio__label:hover,
.fds-radio:not(.fds-radio--readonly, .fds-radio--disabled) > .fds-radio__input:hover + .fds-radio__label {
.fds-radio:not(.fds-radio--readonly) > .fds-radio__label:hover:not(:disabled),
.fds-radio:not(.fds-radio--readonly) > .fds-radio__input:hover:not(:disabled) + .fds-radio__label {
color: var(--fds-semantic-text-action-hover);
}

.fds-radio:not(.fds-radio--readonly, .fds-radio--disabled) > .fds-radio__input:hover:not(:checked) {
.fds-radio:not(.fds-radio--readonly) > .fds-radio__input:hover:not(:checked, :disabled) {
--fds-radio-border-color: var(--fds-semantic-border-input-hover);

box-shadow:
var(--fds-radio-border__hover),
inset 0 0 0 2px var(--fds-radio-border-color);
}

.fds-radio:not(.fds-radio--readonly, .fds-radio--disabled) > .fds-radio__input:hover:checked {
.fds-radio:not(.fds-radio--readonly) > .fds-radio__input:hover:checked:not(:disabled) {
--fds-radio-border-color: var(--fds-semantic-border-input-hover);

box-shadow:
Expand All @@ -135,7 +134,7 @@
inset 0 0 0 6px var(--fds-radio-background);
}

.fds-radio:not(.fds-radio--readonly, .fds-radio--disabled) > .fds-radio__input:hover:checked:focus-visible {
.fds-radio:not(.fds-radio--readonly) > .fds-radio__input:hover:checked:focus-visible:not(:disabled) {
box-shadow:
var(--fds-radio-border__hover),
inset 0 0 0 var(--fds-radio-focus-border-width) var(--fds-semantic-border-focus-boxshadow),
Expand Down
2 changes: 1 addition & 1 deletion packages/css/search.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
z-index: 1;
}

.fds-search--disabled {
.fds-search:has(.fds-search__input:disabled) {
opacity: var(--fds-opacity-disabled);
}

Expand Down
22 changes: 10 additions & 12 deletions packages/css/switch.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,6 @@
cursor: default;
}

.fds-switch--disabled > .fds-switch__input,
.fds-switch--disabled > .fds-switch__label,
.fds-switch--disabled > .fds-switch__track {
cursor: not-allowed;
}

.fds-switch--disabled > .fds-switch__label,
.fds-switch--disabled > .fds-switch__track,
.fds-switch--disabled > .fds-switch__description {
opacity: var(--fds-opacity-disabled);
}

.fds-switch--readonly > .fds-switch__description {
margin-left: var(--fds-spacing-1);
}
Expand Down Expand Up @@ -154,6 +142,16 @@
margin-right: 0;
}

.fds-switch__input:disabled,
.fds-switch:has(.fds-switch__input:disabled) > .fds-switch__label {
cursor: not-allowed;
}

.fds-switch:has(.fds-switch__input:disabled) > .fds-switch__label,
.fds-switch:has(.fds-switch__input:disabled) > .fds-switch__description {
opacity: var(--fds-opacity-disabled);
}

.fds-switch__input:focus-visible + .fds-switch__label .fds-switch__track {
outline: var(--fds-switch-focus-border-width) solid var(--fds-semantic-border-focus-outline);
box-shadow: inset 0 0 0 var(--fds-switch-focus-border-width) var(--fds-semantic-border-focus-boxshadow);
Expand Down
8 changes: 4 additions & 4 deletions packages/css/textarea.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
gap: var(--fds-spacing-2);
}

.fds-textarea__readonly__icon {
.fds-textarea__readonly-icon {
height: 1.2em;
width: 1.2em;
}
Expand Down Expand Up @@ -53,15 +53,15 @@
padding: var(--fds-spacing-4);
}

.fds-textarea--disabled {
.fds-textarea:has(.fds-textarea__input:disabled) {
opacity: var(--fds-opacity-disabled);
}

.fds-textarea--disabled .fds-textarea__input {
.fds-textarea__input:disabled {
cursor: not-allowed;
}

.fds-textarea--readonly .fds-textarea__input {
.fds-textarea__input:read-only {
background: var(--fds-semantic-surface-neutral-subtle);
border-color: var(--fds-semantic-border-neutral-default);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/css/textfield.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
border-radius: var(--fds-border_radius-medium);
}

.fds-textfield--disabled .fds-textfield__input {
.fds-textfield__input:disabled {
cursor: not-allowed;
}

Expand Down Expand Up @@ -101,7 +101,7 @@
margin-top: calc(var(--fds-spacing-2) * -1);
}

.fds-textfield--disabled {
.fds-textfield:has(.fds-textfield__input:disabled) {
opacity: var(--fds-opacity-disabled);
}

Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/form/Checkbox/Checkbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const Checkbox = forwardRef<HTMLInputElement, CheckboxProps>(
className={cl(
'fds-checkbox',
`fds-checkbox--${size}`,
inputProps.disabled && `fds-checkbox--disabled`,
hasError && `fds-checkbox--error`,
readOnly && `fds-checkbox--readonly`,
className,
Expand All @@ -63,6 +62,7 @@ export const Checkbox = forwardRef<HTMLInputElement, CheckboxProps>(
{...omit(['size', 'error', 'indeterminate'], rest)}
{...inputProps}
type='checkbox'
disabled={inputProps.disabled}
aria-checked={rest.indeterminate ? 'mixed' : inputProps.checked}
/>
{children && (
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/form/Fieldset/Fieldset.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ export const Fieldset = forwardRef<HTMLFieldSetElement, FieldsetProps>(
'fds-fieldset',
!hideLegend && 'fds-fieldset--spacing',
readOnly && 'fds-fieldset--readonly',
props?.disabled && 'fds-fieldset--disabled',
className,
)}
disabled={props?.disabled}
ref={ref}
{...rest}
>
Expand Down
1 change: 0 additions & 1 deletion packages/react/src/components/form/NativeSelect/NativeSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ export const NativeSelect = forwardRef<HTMLSelectElement, NativeSelectProps>(
<div
className={cl(
'fds-native-select--container',
disabled && 'fds-native-select--disabled',
readOnly && 'fds-native-select--readonly',
error && 'fds-native-select--error',
)}
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/form/Radio/Radio.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ export const Radio = forwardRef<HTMLInputElement, RadioProps>((props, ref) => {
className={cl(
'fds-radio',
`fds-radio--${size}`,
inputProps.disabled && `fds-radio--disabled`,
hasError && `fds-radio--error`,
readOnly && `fds-radio--readonly`,
className,
Expand All @@ -44,6 +43,7 @@ export const Radio = forwardRef<HTMLInputElement, RadioProps>((props, ref) => {
>
<input
className={'fds-radio__input'}
disabled={inputProps.disabled}
ref={ref}
{...omit(['size', 'error'], rest)}
{...inputProps}
Expand Down
1 change: 1 addition & 0 deletions packages/react/src/components/form/Switch/Switch.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export const Switch = forwardRef<HTMLInputElement, SwitchProps>(
>
<input
className={`fds-switch__input`}
disabled={inputProps.disabled}
ref={ref}
{...omit(['size', 'error'], rest)}
{...inputProps}
Expand Down
6 changes: 3 additions & 3 deletions packages/react/src/components/form/Textarea/Textarea.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ export const Textarea = forwardRef<HTMLTextAreaElement, TextareaProps>(
className={cl(
'fds-textarea',
`fds-textarea--${size}`,
textareaProps.disabled && 'fds-textarea--disabled',
readOnly && `fds-textarea--readonly`,
hasError && `fds-textarea--error`,
className,
)}
Expand All @@ -93,7 +91,7 @@ export const Textarea = forwardRef<HTMLTextAreaElement, TextareaProps>(
{readOnly && (
<PadlockLockedFillIcon
aria-hidden
className='fds-textarea__readonly__icon'
className='fds-textarea__readonly-icon'
/>
)}
<span>{label}</span>
Expand All @@ -119,6 +117,8 @@ export const Textarea = forwardRef<HTMLTextAreaElement, TextareaProps>(
className={cl('fds-textarea__input', `fds-focus`)}
ref={ref}
aria-describedby={describedBy}
disabled={textareaProps.disabled}
readOnly={readOnly}
{...omit(['size', 'error', 'errorId'], rest)}
{...textareaProps}
onChange={(e) => {
Expand Down
Loading
Loading