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.

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

refactor!: update checkbox to use slotted input #2539

Merged
merged 32 commits into from
Sep 21, 2021
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
b1d3e4e
refactor!: use new mixins for checkbox
vursen Sep 13, 2021
816c2ef
fix: update Lumo visual tests
vursen Sep 15, 2021
b4b66bb
fix: update Material visual tests
vursen Sep 15, 2021
ea4fda2
refactor: switch checked state on change event
vursen Sep 17, 2021
df0a675
feat: introduce _shouldSetActive method for ActiveMixin
vursen Sep 17, 2021
0eb56c1
refactor: adapt checkbox to mixins' changes
vursen Sep 17, 2021
127b5d0
fix: update unit tests
vursen Sep 17, 2021
75810d4
fix: prevent input from losing focus while animation
vursen Sep 17, 2021
447f406
test: clean unit tests
vursen Sep 17, 2021
9f9a0da
fix: update type declarations
vursen Sep 17, 2021
e4db172
chore: improve comment writing
vursen Sep 17, 2021
cee506f
fix: delegate name attribute on the input
vursen Sep 17, 2021
254e63a
test: re-organize tests, add more tests
vursen Sep 17, 2021
df4fdd8
refactor: revert AriaLabelMixin
vursen Sep 17, 2021
1c8aeb9
refactor: revert HelperTextMixin
vursen Sep 17, 2021
f12a737
refactor: clean ActiveMixin
vursen Sep 17, 2021
fa9d0e5
fix: import correct Checkbox definition in other components
vursen Sep 17, 2021
dd94a5a
fix: adjust CheckboxGroup to the new Checkbox
vursen Sep 17, 2021
634656f
chore: improve comment writing
vursen Sep 19, 2021
14608aa
chore: improve comment writing, clean package.json
vursen Sep 19, 2021
1b906f6
refactor: set default value in constructor
vursen Sep 19, 2021
914446e
chore: add more code comments
vursen Sep 19, 2021
600ebe3
fix: delegate checked property to input
vursen Sep 19, 2021
743e716
chore: improve comment writing
vursen Sep 20, 2021
69f0985
fix: don't prevent default behaviour for the click event in grid pro
vursen Sep 20, 2021
79fc420
fix: apply code review suggestions
vursen Sep 20, 2021
caf06b6
test: add tests on custom element definition
vursen Sep 20, 2021
6af2f3b
test: re-organize tests on active attribute
vursen Sep 20, 2021
f96ef7a
test: improve test descriptions writing
vursen Sep 20, 2021
9e50772
fix: toggle checkbox in grid pro
vursen Sep 21, 2021
470d836
chore: add more code comments
vursen Sep 21, 2021
8b3859b
chore: improve comment writing
vursen Sep 21, 2021
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
4 changes: 2 additions & 2 deletions packages/checkbox-group/src/vaadin-checkbox-group.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Copyright (c) 2021 Vaadin Ltd.
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { CheckboxElement } from '@vaadin/checkbox/src/vaadin-checkbox.js';
import { Checkbox } from '@vaadin/checkbox/src/vaadin-checkbox.js';
import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js';
import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';

Expand Down Expand Up @@ -116,7 +116,7 @@ declare class CheckboxGroupElement extends ThemableMixin(DirMixin(HTMLElement))

_removeCheckboxFromValue(value: string): void;

_changeSelectedCheckbox(checkbox: CheckboxElement | null): void;
_changeSelectedCheckbox(checkbox: Checkbox | null): void;
web-padawan marked this conversation as resolved.
Show resolved Hide resolved

_containsFocus(): boolean;

Expand Down
20 changes: 9 additions & 11 deletions packages/checkbox-group/src/vaadin-checkbox-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
import { PolymerElement, html } from '@polymer/polymer/polymer-element.js';
import { FlattenedNodesObserver } from '@polymer/polymer/lib/utils/flattened-nodes-observer.js';
import { CheckboxElement } from '@vaadin/checkbox/src/vaadin-checkbox.js';
import { Checkbox } from '@vaadin/checkbox/src/vaadin-checkbox.js';
import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js';
import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';

Expand Down Expand Up @@ -198,15 +198,13 @@ class CheckboxGroupElement extends ThemableMixin(DirMixin(PolymerElement)) {
this.addEventListener('focusin', () => this._setFocused(this._containsFocus()));

this.addEventListener('focusout', (e) => {
// validate when stepping out of the checkbox group
if (
!this._checkboxes.some(
(checkbox) => e.relatedTarget === checkbox || checkbox.shadowRoot.contains(e.relatedTarget)
)
) {
this.validate();
this._setFocused(false);
// Skip if focus is just moved to another checkbox.
if (this._checkboxes.some((checkbox) => checkbox.contains(e.relatedTarget))) {
return;
}

this.validate();
this._setFocused(false);
});

const checkedChangedListener = (e) => {
Expand Down Expand Up @@ -264,7 +262,7 @@ class CheckboxGroupElement extends ThemableMixin(DirMixin(PolymerElement)) {

/** @private */
_filterCheckboxes(nodes) {
return Array.from(nodes).filter((child) => child instanceof CheckboxElement);
return Array.from(nodes).filter((child) => child instanceof Checkbox);
}

/** @private */
Expand Down Expand Up @@ -293,7 +291,7 @@ class CheckboxGroupElement extends ThemableMixin(DirMixin(PolymerElement)) {
}

/**
* @param {CheckboxElement} checkbox
* @param {Checkbox} checkbox
* @protected
*/
_changeSelectedCheckbox(checkbox) {
Expand Down
63 changes: 38 additions & 25 deletions packages/checkbox-group/test/checkbox-group.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { expect } from '@esm-bundle/chai';
import sinon from 'sinon';
import { fixtureSync, focusin, focusout } from '@vaadin/testing-helpers';
import { expect } from '@esm-bundle/chai';
import { sendKeys } from '@web/test-runner-commands';
import { fixtureSync } from '@vaadin/testing-helpers';
import '@polymer/polymer/lib/elements/dom-bind.js';
import '../vaadin-checkbox-group.js';

Expand Down Expand Up @@ -139,26 +140,33 @@ describe('vaadin-checkbox-group', () => {
expect(checkboxes[2].checked).to.be.false;
});

it('should set focused attribute on focusin event dispatched', () => {
focusin(checkboxes[0]);
it('should set focused attribute on Tab', async () => {
// Focus on the first checkbox.
await sendKeys({ press: 'Tab' });

expect(checkboxes[0].hasAttribute('focused')).to.be.true;
expect(group.hasAttribute('focused')).to.be.true;
});

it('should not set focused attribute on focusin event dispatched when disabled', () => {
group.disabled = true;
focusin(checkboxes[0]);
expect(group.hasAttribute('focused')).to.be.false;
});
it('should remove focused attribute on Shift+Tab', async () => {
// Focus on the first checkbox.
await sendKeys({ press: 'Tab' });

it('should remove focused attribute on checkbox focusout', () => {
focusin(checkboxes[0]);
focusout(checkboxes[0]);
// Move focus out of the checkbox group.
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });

expect(checkboxes[0].hasAttribute('focused')).to.be.false;
expect(group.hasAttribute('focused')).to.be.false;
});

it('should remove focused attribute on checkbox-group focusout', () => {
focusin(checkboxes[0]);
focusout(group);
it('should not set focused attribute on Tab when disabled', async () => {
group.disabled = true;
// Try to focus on the first checkbox.
await sendKeys({ press: 'Tab' });

expect(checkboxes[0].hasAttribute('focused')).to.be.false;
expect(group.hasAttribute('focused')).to.be.false;
});

Expand Down Expand Up @@ -330,23 +338,28 @@ describe('validation', () => {
expect(group.hasAttribute('invalid')).to.be.false;
});

it('should pass validation and set invalid when field is required and user blurs out of the group', () => {
it('should run validation and set invalid when field is required and user blurs out of the group', async () => {
group.required = true;
focusout(group, document.body);

// Focus on the first checkbox.
await sendKeys({ press: 'Tab' });
// Move focus out of the checkbox group.
await sendKeys({ down: 'Shift' });
await sendKeys({ press: 'Tab' });
await sendKeys({ up: 'Shift' });

expect(group.invalid).to.be.true;
});

it('should not run validation while user is tabbing between checkboxes inside of the group', () => {
it('should not run validation while user is tabbing between checkboxes inside of the group', async () => {
group.required = true;
const spy = sinon.spy(group, 'validate');
focusout(group, checkboxes[1]);
expect(spy.called).to.be.false;
});

it('should not run validation while user is tabbing between checkboxes and focus moves to native checkbox', () => {
group.required = true;
const spy = sinon.spy(group, 'validate');
focusout(group, checkboxes[1].focusElement);
// Focus on the first checkbox.
await sendKeys({ press: 'Tab' });
// Focus on the second checkbox.
await sendKeys({ press: 'Tab' });

expect(spy.called).to.be.false;
});

Expand Down
2 changes: 1 addition & 1 deletion packages/checkbox/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"dependencies": {
"@polymer/polymer": "^3.0.0",
"@vaadin/component-base": "^22.0.0-alpha6",
"@vaadin/vaadin-control-state-mixin": "^22.0.0-alpha6",
"@vaadin/field-base": "^22.0.0-alpha6",
"@vaadin/vaadin-lumo-styles": "^22.0.0-alpha6",
"@vaadin/vaadin-material-styles": "^22.0.0-alpha6",
"@vaadin/vaadin-themable-mixin": "^22.0.0-alpha6"
Expand Down
80 changes: 35 additions & 45 deletions packages/checkbox/src/vaadin-checkbox.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@
* Copyright (c) 2021 Vaadin Ltd.
* This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
*/
import { GestureEventListeners } from '@polymer/polymer/lib/mixins/gesture-event-listeners.js';
import { ControlStateMixin } from '@vaadin/vaadin-control-state-mixin/vaadin-control-state-mixin.js';
import { ActiveMixin } from '@vaadin/component-base/src/active-mixin.js';
import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
import { AriaLabelMixin } from '@vaadin/field-base/src/aria-label-mixin.js';
import { CheckedMixin } from '@vaadin/field-base/src/checked-mixin.js';
import { InputSlotMixin } from '@vaadin/field-base/src/input-slot-mixin.js';
import { SlotLabelMixin } from '@vaadin/field-base/src/slot-label-mixin.js';
import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';

/**
Expand All @@ -18,95 +21,82 @@ export type CheckboxCheckedChangedEvent = CustomEvent<{ value: boolean }>;
*/
export type CheckboxIndeterminateChangedEvent = CustomEvent<{ value: boolean }>;

export interface CheckboxElementEventMap {
export interface CheckboxCustomEventMap {
'checked-changed': CheckboxCheckedChangedEvent;

'indeterminate-changed': CheckboxIndeterminateChangedEvent;
}

export interface CheckboxEventMap extends HTMLElementEventMap, CheckboxElementEventMap {}
export interface CheckboxEventMap extends HTMLElementEventMap, CheckboxCustomEventMap {}

/**
* `<vaadin-checkbox>` is a Web Component for customized checkboxes.
* `<vaadin-checkbox>` is an input field representing a binary choice.
*
* ```html
* <vaadin-checkbox>
* Make my profile visible
* </vaadin-checkbox>
* <vaadin-checkbox>I accept the terms and conditions</vaadin-checkbox>
* ```
*
* ### Styling
*
* The following shadow DOM parts are available for styling:
*
* Part name | Description
* ------------------|----------------
* `checkbox` | The wrapper element for the native <input type="checkbox">
* `label` | The wrapper element in which the component's children, namely the label, is slotted
* Part name | Description
* ------------|----------------
* `container` | The container element
* `checkbox` | The wrapper element that contains slotted `<input type="checkbox">`
* `label` | The wrapper element that contains slotted `<label>`
*
* The following state attributes are available for styling:
*
* Attribute | Description | Part name
* -------------|-------------|--------------
* `active` | Set when the checkbox is pressed down, either with mouse, touch or the keyboard. | `:host`
* `disabled` | Set when the checkbox is disabled. | `:host`
* `focus-ring` | Set when the checkbox is focused using the keyboard. | `:host`
* `focused` | Set when the checkbox is focused. | `:host`
* `indeterminate` | Set when the checkbox is in indeterminate mode. | `:host`
* `checked` | Set when the checkbox is checked. | `:host`
* `empty` | Set when there is no label provided. | `label`
* Attribute | Description | Part name
* ----------------|-------------|--------------
* `active` | Set when the checkbox is pressed down, either with mouse, touch or the keyboard. | `:host`
* `disabled` | Set when the checkbox is disabled. | `:host`
* `focus-ring` | Set when the checkbox is focused using the keyboard. | `:host`
* `focused` | Set when the checkbox is focused. | `:host`
* `indeterminate` | Set when the checkbox is in the indeterminate state. | `:host`
* `checked` | Set when the checkbox is checked. | `:host`
* `has-label` | Set when the checkbox has a label. | `:host`
*
* See [Styling Components](https://vaadin.com/docs/latest/ds/customization/styling-components) documentation.
*
* @fires {Event} change - Fired when the user commits a value change.
* @fires {CustomEvent} checked-changed - Fired when the `checked` property changes.
* @fires {CustomEvent} indeterminate-changed - Fired when the `indeterminate` property changes.
*/
declare class CheckboxElement extends ElementMixin(
ControlStateMixin(ThemableMixin(GestureEventListeners(HTMLElement)))
declare class Checkbox extends SlotLabelMixin(
CheckedMixin(InputSlotMixin(AriaLabelMixin(ActiveMixin(ElementMixin(ThemableMixin(HTMLElement))))))
) {
readonly focusElement: HTMLInputElement;

/**
* Name of the element.
*/
name: string;

/**
* True if the checkbox is checked.
*/
checked: boolean;

/**
* Indeterminate state of the checkbox when it's neither checked nor unchecked, but undetermined.
* True if the checkbox is in the indeterminate state which means
* it is not possible to say whether it is checked or unchecked.
* The state is reset once the user explicitly switches the checkbox.
*
* https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/checkbox#Indeterminate_state_checkboxes
*/
indeterminate: boolean;

/**
* The value given to the data submitted with the checkbox's name to the server when the control is inside a form.
* The name of the checkbox.
*/
value: string | null | undefined;

_toggleChecked(): void;
name: string;

addEventListener<K extends keyof CheckboxEventMap>(
type: K,
listener: (this: CheckboxElement, ev: CheckboxEventMap[K]) => void,
listener: (this: Checkbox, ev: CheckboxEventMap[K]) => void,
options?: boolean | AddEventListenerOptions
): void;

removeEventListener<K extends keyof CheckboxEventMap>(
type: K,
listener: (this: CheckboxElement, ev: CheckboxEventMap[K]) => void,
listener: (this: Checkbox, ev: CheckboxEventMap[K]) => void,
options?: boolean | EventListenerOptions
): void;
}

declare global {
interface HTMLElementTagNameMap {
'vaadin-checkbox': CheckboxElement;
'vaadin-checkbox': Checkbox;
}
}

export { CheckboxElement };
export { Checkbox };
Loading