Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions core/src/components/checkbox/checkbox.ios.scss
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,26 @@
:host(.checkbox-disabled) {
opacity: $checkbox-ios-disabled-opacity;
}

// iOS Checkbox: Keyboard Focus
// -----------------------------------------

:host(.ion-focused) .native-wrapper::after {
@include border-radius(50%);

display: block;
position: absolute;

width: 36px;
height: 36px;

transform: translate(-50%, -50%);

background: $checkbox-ios-background-color-focused;

content: "";
opacity: 0.2;

inset-block-start: 50%;
inset-inline-start: 50%;
}
3 changes: 3 additions & 0 deletions core/src/components/checkbox/checkbox.ios.vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ $checkbox-ios-disabled-opacity: $form-control-ios-disabled-opacity;

/// @prop - Checkmark width of the checkbox icon
$checkbox-ios-icon-checkmark-width: 1.5px;

/// @prop - Background color of focus indicator for checkbox when focused
$checkbox-ios-background-color-focused: ion-color(primary, tint);
23 changes: 23 additions & 0 deletions core/src/components/checkbox/checkbox.md.scss
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,26 @@
:host(.checkbox-disabled) .native-wrapper {
opacity: $checkbox-md-icon-disabled-opacity;
}

// Material Design Checkbox: Keyboard Focus
// -----------------------------------------

:host(.ion-focused) .native-wrapper::after {
@include border-radius(50%);

display: block;
position: absolute;

width: 36px;
height: 36px;

transform: translate(-50%, -50%);

background: $checkbox-md-background-color-focused;

content: "";
opacity: 0.2;

inset-block-start: 50%;
inset-inline-start: 50%;
}
3 changes: 3 additions & 0 deletions core/src/components/checkbox/checkbox.md.vars.scss
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ $checkbox-md-transition-duration: 180ms;
/// @prop - Transition easing of the checkbox
$checkbox-md-transition-easing: cubic-bezier(.4, 0, .2, 1);

/// @prop - Background color of focus indicator for checkbox when focused
$checkbox-md-background-color-focused: ion-color(primary, tint);

/// @prop - Opacity of the disabled checkbox
/// This value is used because the checkbox color is set to
/// `rgb(0, 0, 0, 0.60)` when enabled and we need it to be
Expand Down
2 changes: 2 additions & 0 deletions core/src/components/checkbox/checkbox.scss

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to remove the large outline that appears in the browser on focus:

Image

Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ input {
.native-wrapper {
display: flex;

position: relative;

align-items: center;
}

Expand Down
33 changes: 30 additions & 3 deletions core/src/components/checkbox/checkbox.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ComponentInterface, EventEmitter } from '@stencil/core';
import { Build, Component, Element, Event, Host, Method, Prop, State, h } from '@stencil/core';
import { Build, Component, Element, Event, Host, Method, Prop, State, forceUpdate, h } from '@stencil/core';
import { checkInvalidState } from '@utils/forms';
import type { Attributes } from '@utils/helpers';
import { inheritAriaAttributes, renderHiddenInput } from '@utils/helpers';
Expand Down Expand Up @@ -37,6 +37,7 @@ export class Checkbox implements ComponentInterface {
private errorTextId = `${this.inputId}-error-text`;
private inheritedAttributes: Attributes = {};
private validationObserver?: MutationObserver;
private itemFocusObserver?: MutationObserver;

@Element() el!: HTMLIonCheckboxElement;

Expand Down Expand Up @@ -199,6 +200,22 @@ export class Checkbox implements ComponentInterface {
// Always set initial state
this.isInvalid = checkInvalidState(el);
this.hasLabelContent = this.el.textContent !== '';

// The item toggles `item-multiple-inputs` after this control renders and as
// inputs are added or removed. Re-render when it flips so the focus
// indicator stays in sync.
const item = el.closest('ion-item');
if (item && Build.isBrowser && typeof MutationObserver !== 'undefined') {
let wasMultipleInputs = item.classList.contains('item-multiple-inputs');
this.itemFocusObserver = new MutationObserver(() => {
const isMultipleInputs = item.classList.contains('item-multiple-inputs');
if (isMultipleInputs !== wasMultipleInputs) {
wasMultipleInputs = isMultipleInputs;
forceUpdate(this);
}
});
this.itemFocusObserver.observe(item, { attributes: true, attributeFilter: ['class'] });
}
}

componentWillLoad() {
Expand All @@ -210,11 +227,15 @@ export class Checkbox implements ComponentInterface {
}

disconnectedCallback() {
// Clean up validation observer to prevent memory leaks.
// Clean up observers to prevent memory leaks.
if (this.validationObserver) {
this.validationObserver.disconnect();
this.validationObserver = undefined;
}
if (this.itemFocusObserver) {
this.itemFocusObserver.disconnect();
this.itemFocusObserver = undefined;
}
}

/** @internal */
Expand Down Expand Up @@ -338,6 +359,8 @@ export class Checkbox implements ComponentInterface {
} = this;
const mode = getIonMode(this);
const path = getSVGPath(mode, indeterminate);
const inItem = hostContext('ion-item', el);
const inMultipleInputsItem = hostContext('ion-item.item-multiple-inputs', el);

renderHiddenInput(true, el, name, checked ? value : '', disabled);

Expand All @@ -360,11 +383,15 @@ export class Checkbox implements ComponentInterface {
onClick={this.onClick}
class={createColorClasses(color, {
[mode]: true,
'in-item': hostContext('ion-item', el),
'in-item': inItem,
'checkbox-checked': checked,
'checkbox-disabled': disabled,
'checkbox-indeterminate': indeterminate,
interactive: true,
// Focus styling should not apply when the checkbox is in an item,
// since the item handles the focus indicator instead. The exception
// is a multi-input item, which has no single indicator of its own.
'ion-focusable': !inItem || inMultipleInputsItem,
[`checkbox-justify-${justify}`]: justify !== undefined,
[`checkbox-alignment-${alignment}`]: alignment !== undefined,
[`checkbox-label-placement-${labelPlacement}`]: true,
Expand Down
64 changes: 64 additions & 0 deletions core/src/components/checkbox/test/a11y/checkbox.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,70 @@ configs({ directions: ['ltr'], palettes: ['light', 'dark'] }).forEach(({ title,
});
});

/**
* These tests assert the `ion-focusable` gating class the component controls,
* not the rendered focus ring. `ion-focused` is not asserted directly because
* it depends on keyboard-mode detection, which is unreliable on WebKit. The
* gating logic does not vary across modes.
*/
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
test.describe(title('checkbox: focus indicator'), () => {
test('standalone checkbox should be focusable', async ({ page }) => {
await page.setContent(
`
<ion-app>
<ion-checkbox aria-label="Checkbox">Checkbox</ion-checkbox>
</ion-app>
`,
config
);

const checkbox = page.locator('ion-checkbox');
await expect(checkbox).toHaveClass(/ion-focusable/);
});

test('checkbox in a single-input item should not show its own focus indicator', async ({ page }) => {
await page.setContent(
`
<ion-app>
<ion-item>
<ion-checkbox>Checkbox</ion-checkbox>
</ion-item>
</ion-app>
`,
config
);

// The item owns the focus indicator for single-input items, so the
// checkbox must not become focusable itself.
const checkbox = page.locator('ion-checkbox');
const item = page.locator('ion-item');
await expect(checkbox).not.toHaveClass(/ion-focusable/);
await expect(item).toHaveClass(/ion-focusable/);
});

test('checkbox in a multi-input item should be focusable', async ({ page }) => {
await page.setContent(
`
<ion-app>
<ion-item>
<ion-checkbox>Checkbox 1</ion-checkbox>
<ion-checkbox>Checkbox 2</ion-checkbox>
</ion-item>
</ion-app>
`,
config
);

// Multi-input items do not draw a single focus indicator, so each control
// must be able to show its own.
const checkboxes = page.locator('ion-checkbox');
await expect(checkboxes.nth(0)).toHaveClass(/ion-focusable/);
await expect(checkboxes.nth(1)).toHaveClass(/ion-focusable/);
});
});
});

configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => {
test.describe(title('checkbox: a11y'), () => {
test.describe(title('checkbox: font scaling'), () => {
Expand Down
30 changes: 27 additions & 3 deletions core/src/components/radio/radio.tsx

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we remove these styles from radio for ios:

Image

It looks way better without them:

Image

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ComponentInterface, EventEmitter } from '@stencil/core';
import { Component, Element, Event, Host, Method, Prop, State, Watch, h } from '@stencil/core';
import { Build, Component, Element, Event, Host, Method, Prop, State, Watch, forceUpdate, h } from '@stencil/core';
import { isOptionSelected } from '@utils/forms';
import { addEventListener, removeEventListener } from '@utils/helpers';
import { createColorClasses, hostContext } from '@utils/theme';
Expand Down Expand Up @@ -27,6 +27,7 @@ import type { Color } from '../../interface';
export class Radio implements ComponentInterface {
private inputId = `ion-rb-${radioButtonIds++}`;
private radioGroup: HTMLIonRadioGroupElement | null = null;
private itemFocusObserver?: MutationObserver;

@Element() el!: HTMLIonRadioElement;

Expand Down Expand Up @@ -150,6 +151,22 @@ export class Radio implements ComponentInterface {
this.updateState();
addEventListener(radioGroup, 'ionValueChange', this.updateState);
}

// The item toggles `item-multiple-inputs` after this control renders and as
// inputs are added or removed. Re-render when it flips so the focus
// indicator stays in sync.
const item = this.el.closest('ion-item');
if (item && Build.isBrowser && typeof MutationObserver !== 'undefined') {
let wasMultipleInputs = item.classList.contains('item-multiple-inputs');
this.itemFocusObserver = new MutationObserver(() => {
const isMultipleInputs = item.classList.contains('item-multiple-inputs');
if (isMultipleInputs !== wasMultipleInputs) {
wasMultipleInputs = isMultipleInputs;
forceUpdate(this);
}
});
this.itemFocusObserver.observe(item, { attributes: true, attributeFilter: ['class'] });
}
}

disconnectedCallback() {
Expand All @@ -158,6 +175,10 @@ export class Radio implements ComponentInterface {
removeEventListener(radioGroup, 'ionValueChange', this.updateState);
this.radioGroup = null;
}
if (this.itemFocusObserver) {
this.itemFocusObserver.disconnect();
this.itemFocusObserver = undefined;
}
}

private updateState = () => {
Expand Down Expand Up @@ -216,6 +237,7 @@ export class Radio implements ComponentInterface {
const { checked, disabled, color, el, justify, labelPlacement, hasLabel, buttonTabindex, alignment } = this;
const mode = getIonMode(this);
const inItem = hostContext('ion-item', el);
const inMultipleInputsItem = hostContext('ion-item.item-multiple-inputs', el);

return (
<Host
Expand All @@ -230,9 +252,11 @@ export class Radio implements ComponentInterface {
[`radio-justify-${justify}`]: justify !== undefined,
[`radio-alignment-${alignment}`]: alignment !== undefined,
[`radio-label-placement-${labelPlacement}`]: true,
// Focus and active styling should not apply when the radio is in an item
// Focus/active styling should not apply in an item, since the item
// handles it. The exception is a multi-input item, which has no single
// indicator of its own; active styling stays suppressed there regardless.
'ion-activatable': !inItem,
'ion-focusable': !inItem,
'ion-focusable': !inItem || inMultipleInputsItem,
})}
role="radio"
aria-checked={checked ? 'true' : 'false'}
Expand Down
72 changes: 72 additions & 0 deletions core/src/components/radio/test/a11y/radio.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,78 @@ configs({ directions: ['ltr'], palettes: ['light', 'dark'] }).forEach(({ title,
});
});

/**
* These tests assert the `ion-focusable` gating class the component controls,
* not the rendered focus ring. `ion-focused` is not asserted directly because
* it depends on keyboard-mode detection, which is unreliable on WebKit. The
* gating logic does not vary across modes.
*/
configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, config }) => {
test.describe(title('radio: focus indicator'), () => {
test('standalone radio should be focusable', async ({ page }) => {
await page.setContent(
`
<ion-app>
<ion-radio-group value="a">
<ion-radio value="a" aria-label="Radio">Radio</ion-radio>
</ion-radio-group>
</ion-app>
`,
config
);

const radio = page.locator('ion-radio');
await expect(radio).toHaveClass(/ion-focusable/);
});

test('radio in a single-input item should not show its own focus indicator', async ({ page }) => {
await page.setContent(
`
<ion-app>
<ion-radio-group value="a">
<ion-item>
<ion-radio value="a">Radio</ion-radio>
</ion-item>
</ion-radio-group>
</ion-app>
`,
config
);

// The item owns the focus indicator for single-input items, so the radio
// must not become focusable itself.
const radio = page.locator('ion-radio');
const item = page.locator('ion-item');
await expect(radio).not.toHaveClass(/ion-focusable/);
await expect(item).toHaveClass(/ion-focusable/);
});

test('radio in a multi-input item should be focusable', async ({ page }) => {
await page.setContent(
`
<ion-app>
<ion-item>
<ion-radio-group value="a">
<ion-radio value="a">Radio 1</ion-radio>
</ion-radio-group>
<ion-radio-group value="b">
<ion-radio value="b">Radio 2</ion-radio>
</ion-radio-group>
</ion-item>
</ion-app>
`,
config
);

// Multi-input items do not draw a single focus indicator, so each control
// must be able to show its own.
const radios = page.locator('ion-radio');
await expect(radios.nth(0)).toHaveClass(/ion-focusable/);
await expect(radios.nth(1)).toHaveClass(/ion-focusable/);
});
});
});

/**
* This behavior does not vary across directions
*/
Expand Down
Loading
Loading