Skip to content

Commit

Permalink
chore(checkbox): address pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
samrichardsontylertech committed Oct 30, 2023
1 parent ab06c17 commit faf0c71
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 55 deletions.
40 changes: 26 additions & 14 deletions src/lib/checkbox/checkbox-adapter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getShadowElement, toggleAttribute } from '@tylertech/forge-core';
import { BaseAdapter, IBaseAdapter, INPUT_PROPERTIES, InputAdapter, cloneAttributes, cloneProperties, cloneValidationMessage, forwardAttributes } from '../core';
import { BaseAdapter, IBaseAdapter, INPUT_PROPERTIES, SlottedElementAdapter, cloneAttributes, cloneProperties, cloneValidationMessage, forwardAttributes } from '../core';
import { StateLayerComponent } from '../state-layer';
import { ICheckboxComponent } from './checkbox';
import { CHECKBOX_CONSTANTS, CheckboxLabelPosition, CheckboxState } from './checkbox-constants';
Expand Down Expand Up @@ -28,7 +28,7 @@ export class CheckboxAdapter extends BaseAdapter<ICheckboxComponent> implements
private readonly _labelElement: HTMLElement;
private readonly _inputSlotElement: HTMLSlotElement;
private readonly _stateLayerElement: StateLayerComponent;
private readonly _inputAdapter: InputAdapter;
private readonly _inputAdapter: SlottedElementAdapter<HTMLInputElement>;
private _forwardObserver?: MutationObserver;

private get _activeInputElement(): HTMLInputElement {
Expand All @@ -43,19 +43,31 @@ export class CheckboxAdapter extends BaseAdapter<ICheckboxComponent> implements
this._labelElement = getShadowElement(component, CHECKBOX_CONSTANTS.selectors.LABEL);
this._inputSlotElement = getShadowElement(component, CHECKBOX_CONSTANTS.selectors.INPUT_SLOT) as HTMLSlotElement;
this._stateLayerElement = getShadowElement(component, CHECKBOX_CONSTANTS.selectors.STATE_LAYER) as StateLayerComponent;
this._inputAdapter = new InputAdapter();
this._inputAdapter = new SlottedElementAdapter();
}

public initialize(): void {
this._inputAdapter.initialize(this._inputElement, (newEl, oldEl) => {
if (oldEl) {
cloneAttributes(oldEl, newEl, ['type', 'checked', 'aria-readonly']);
cloneProperties(oldEl, newEl, INPUT_PROPERTIES);
cloneValidationMessage(oldEl, newEl);
}

this._forwardObserver?.disconnect();
this._initializeForwardObserver(newEl);
const slottedInput = this._component.querySelector(':is(input[type=checkbox]:not([slot]), [slot=input])') as HTMLInputElement;
if (slottedInput) {
slottedInput.slot = 'input';
}
this.observeInput(slottedInput ?? this._inputElement);
if (!slottedInput) {
this.initializeInput();
}
}

public initializeInput(): void {
this._forwardObserver?.disconnect();
this._initializeForwardObserver(this._activeInputElement);
}

public observeInput(el: HTMLInputElement = this._inputElement): void {
this._inputAdapter.initialize(el, (newEl, oldEl) => {
cloneAttributes(oldEl, newEl, ['type', 'checked', 'aria-readonly']);
cloneProperties(oldEl, newEl, INPUT_PROPERTIES);
cloneValidationMessage(oldEl, newEl);
this.initializeInput();
});
}

Expand Down Expand Up @@ -111,9 +123,9 @@ export class CheckboxAdapter extends BaseAdapter<ICheckboxComponent> implements
public detectInputElement(): void {
const inputElement = this._inputSlotElement.assignedElements()[0] as HTMLInputElement;
if (inputElement) {
this._inputAdapter.attachInput(inputElement);
this._inputAdapter.attachElement(inputElement);
} else {
this._inputAdapter.attachInput(this._inputElement);
this._inputAdapter.attachElement(this._inputElement);
}
}

Expand Down
12 changes: 6 additions & 6 deletions src/lib/checkbox/checkbox.html
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
<template>
<div class="forge-checkbox" part="checkbox">
<div id="container" class="container" part="input-container">
<div id="container" class="container">
<slot name="input">
<input id="input" class="input" type="checkbox" part="input" />
<input id="input" class="input" type="checkbox" />
</slot>
<div class="background" part="background">
<svg class="icon icon--checked" aria-hidden="true" viewBox="0 0 24 24" part="checkmark">
<path class="path path--checkmark" d="M1.73,12.91 8.1,19.28 22.79,4.59" part="checkmark-path"></path>
<path d="M1.73,12.91 8.1,19.28 22.79,4.59"></path>
</svg>
<svg class="icon icon--indeterminate" aria-hidden="true" viewBox="0 0 24 24" part="mixedmark">
<line x1="2" y1="12" x2="22" y2="12" part="mixedmark-path"></line>
<line x1="2" y1="12" x2="22" y2="12"></line>
</svg>
<forge-focus-indicator target="container" part="focus-indicator"></forge-focus-indicator>
<forge-focus-indicator target="container" exportparts="indicator:focus-indicator"></forge-focus-indicator>
</div>
<forge-state-layer target="container" part="state-layer"></forge-state-layer>
<forge-state-layer target="container" exportparts="surface:state-layer"></forge-state-layer>
</div>
<label id="label" for="input" class="label" part="label">
<slot></slot>
Expand Down
1 change: 0 additions & 1 deletion src/lib/checkbox/checkbox.scss
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
@use './core';
@use './configuration';
@use '../core/styles/tokens/checkbox/tokens';
@use '../focus-indicator';
@use '../state-layer';

Expand Down
2 changes: 1 addition & 1 deletion src/lib/core/styles/tokens/checkbox/_tokens.scss
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ $tokens: (
width: utils.module-ref(checkbox, width, size),
height: utils.module-ref(checkbox, height, size),
unchecked-border-width: utils.module-ref(checkbox, unchecked-border-width, border-width),
unchecked-border-color: utils.module-val(checkbox, unchecked-border-color, theme.variable(surface-container-high)), // TODO: check out surface-container
unchecked-border-color: utils.module-val(checkbox, unchecked-border-color, theme.variable(surface-container-high)),
shape: utils.module-val(checkbox, shape, shape.variable(small)),
elevation: utils.module-val(checkbox, elevation, elevation.value(0)),

Expand Down
47 changes: 25 additions & 22 deletions src/lib/core/utils/reflect-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,16 @@ export function cloneValidationMessage(from: HTMLInputElement, to: HTMLInputElem
}

/**
* A utility class for switching between input elements.
* A utility class for switching between elements.
*/
export class InputAdapter {
private _el: HTMLInputElement;
private _attachCallback: (newEl: HTMLInputElement, oldEl?: HTMLInputElement) => void;
export class SlottedElementAdapter<T extends HTMLElement = HTMLElement> {
private _el: T;
private _attachCallback: (newEl: T, oldEl: T) => void;

/**
* Returns the input element associated with this adapter.
* Returns the element associated with this adapter.
*/
public get el(): HTMLInputElement {
public get el(): T {
return this._el;
}

Expand All @@ -182,7 +182,7 @@ export class InputAdapter {
* @param to - The element to clone attributes to.
* @param attributes - The names of the attributes to clone.
*/
public static cloneAttributes(from: HTMLInputElement, to: HTMLInputElement, attributes: string[]): void {
public static cloneAttributes(from: HTMLElement, to: HTMLElement, attributes: string[]): void {
cloneAttributes(from, to, attributes);
}

Expand All @@ -192,38 +192,41 @@ export class InputAdapter {
* @param from - The element to clone properties from.
* @param to - The element to clone properties to.
*/
public static cloneProperties(from: HTMLInputElement, to: HTMLInputElement): void {
cloneProperties(from, to, INPUT_PROPERTIES);
public static cloneProperties(from: HTMLElement, to: HTMLElement, properties: (keyof HTMLElement)[]): void {
cloneProperties(from, to, properties);
}

/**
* Clones the validation message from one input element to another.
* Clones the validation message from one element to another.
*
* @param from - The input element to clone the validation message from.
* @param to - The input element to clone the validation message to.
* @param from - The element to clone the validation message from.
* @param to - The element to clone the validation message to.
*/
public static cloneValidationMessage(from: HTMLInputElement, to: HTMLInputElement): void {
cloneValidationMessage(from, to);
public static cloneValidationMessage(from: HTMLElement, to: HTMLElement): void {
if (Object.hasOwnProperty.call(from, 'validationMessage') && Object.hasOwnProperty.call(to, 'validationMessage')) {
cloneValidationMessage(from as HTMLInputElement, to as HTMLInputElement);
} else {
console.warn('cloneValidationMessage() requires both elements to be input elements.');
}
}

/**
* Initializes the adapter with an initial input element and attach callback.
* Initializes the adapter with an initial element and attach callback.
*
* @param el - The input element to associate with the adapter.
* @param attachCallback - The callback to invoke when attaching the input element.
* @param el - The element to associate with the adapter.
* @param attachCallback - The callback to invoke when attaching the element.
*/
public initialize(el: HTMLInputElement, attachCallback: (newEl: HTMLInputElement, oldEl?: HTMLInputElement) => void): void {
public initialize(el: T, attachCallback: (newEl: T, oldEl: T) => void): void {
this._attachCallback = attachCallback;
this._attachCallback(el, this._el);
this._el = el;
}

/**
* Replaces the attached input element.
* Replaces the attached element.
*
* @param el - The new input element to attach.
* @param el - The new element to attach.
*/
public attachInput(el: HTMLInputElement): void {
public attachElement(el: T): void {
this._attachCallback(el, this._el);
this._el = el;
}
Expand Down
1 change: 0 additions & 1 deletion src/lib/list/list/list-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export class ListAdapter extends BaseAdapter<IListComponent> implements IListAda
const listItems = this._getFocusableListItems();
if (listItems.length) {
const focusedListItemIndex = listItems.findIndex(item => item.matches(':focus'));
console.log('focusedListItemIndex', focusedListItemIndex);
const nextIndex = focusedListItemIndex < listItems.length - 1 ? focusedListItemIndex + 1 : 0;
if (nextIndex <= listItems.length - 1) {
listItems[nextIndex].focus({ preventScroll: true});
Expand Down
4 changes: 2 additions & 2 deletions src/lib/slider/slider.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
<div class="handle-container-block">
<div class="handle-container">
<div class="handle end" part="handle-end">
<forge-state-layer exportparts="surface:state-layer__surface" target="end"></forge-state-layer>
<forge-focus-indicator exportparts="indicator:focus-indicator__indicator" target="end"></forge-focus-indicator>
<forge-state-layer exportparts="surface:state-layer" target="end"></forge-state-layer>
<forge-focus-indicator exportparts="indicator:focus-indicator" target="end"></forge-focus-indicator>
<div class="handle-thumb" part="handle-end-thumb"></div>
<div class="handle-label" part="handle-end-label">
<span class="handle-label-content" part="handle-end-label-content"></span>
Expand Down
16 changes: 8 additions & 8 deletions src/lib/switch/switch-adapter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getShadowElement, toggleAttribute, toggleClass } from '@tylertech/forge-core';
import { BaseAdapter, IBaseAdapter, InputAdapter, forwardAttributes } from '../core';
import { BaseAdapter, IBaseAdapter, SlottedElementAdapter, forwardAttributes } from '../core';
import { StateLayerComponent } from '../state-layer';
import { ISwitchComponent } from './switch';
import { SWITCH_CONSTANTS, SwitchIconVisibility, SwitchLabelPosition } from './switch-constants';
Expand Down Expand Up @@ -30,7 +30,7 @@ export class SwitchAdapter extends BaseAdapter<ISwitchComponent> implements ISwi
private readonly _iconOffElement: HTMLElement;
private readonly _inputSlotElement: HTMLSlotElement;
private readonly _stateLayerElement: StateLayerComponent;
private readonly _inputAdapter: InputAdapter;
private readonly _inputAdapter: SlottedElementAdapter;
private _forwardObserver?: MutationObserver;

private get _activeInputElement(): HTMLInputElement {
Expand All @@ -46,15 +46,15 @@ export class SwitchAdapter extends BaseAdapter<ISwitchComponent> implements ISwi
this._iconOnElement = getShadowElement(component, SWITCH_CONSTANTS.selectors.ICON_ON);
this._iconOffElement = getShadowElement(component, SWITCH_CONSTANTS.selectors.ICON_OFF);
this._stateLayerElement = getShadowElement(component, SWITCH_CONSTANTS.selectors.STATE_LAYER) as StateLayerComponent;
this._inputAdapter = new InputAdapter();
this._inputAdapter = new SlottedElementAdapter();
}

public initialize(): void {
this._inputAdapter.initialize(this._inputElement, (newEl, oldEl) => {
if (oldEl) {
InputAdapter.cloneAttributes(oldEl, newEl, ['type', 'role', 'checked', 'aria-readonly']);
InputAdapter.cloneProperties(oldEl, newEl);
InputAdapter.cloneValidationMessage(oldEl, newEl);
SlottedElementAdapter.cloneAttributes(oldEl, newEl, ['type', 'role', 'checked', 'aria-readonly']);
SlottedElementAdapter.cloneProperties(oldEl, newEl);
SlottedElementAdapter.cloneValidationMessage(oldEl, newEl);
}

this._forwardObserver?.disconnect();
Expand Down Expand Up @@ -121,9 +121,9 @@ export class SwitchAdapter extends BaseAdapter<ISwitchComponent> implements ISwi
public detectInputElement(): void {
const inputElement = this._inputSlotElement.assignedElements()[0] as HTMLInputElement;
if (inputElement) {
this._inputAdapter.attachInput(inputElement);
this._inputAdapter.attachElement(inputElement);
} else {
this._inputAdapter.attachInput(this._inputElement);
this._inputAdapter.attachElement(this._inputElement);
}
}

Expand Down

0 comments on commit faf0c71

Please sign in to comment.