Skip to content

Commit c984a66

Browse files
Joren BroekemadaKmoR
Joren Broekema
and
daKmoR
committed
fix(fieldset): manage when to show error messages
Co-authored-by: Thomas Allmer <thomas.allmer@ing.com>
1 parent 65a94c9 commit c984a66

File tree

14 files changed

+536
-324
lines changed

14 files changed

+536
-324
lines changed

‎packages/checkbox-group/src/LionCheckboxGroup.js‎

Lines changed: 0 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,6 @@
11
import { LionFieldset } from '@lion/fieldset';
22

33
export class LionCheckboxGroup extends LionFieldset {
4-
constructor() {
5-
super();
6-
this._checkboxGroupTouched = false;
7-
this._setTouchedAndPrefilled = this._setTouchedAndPrefilled.bind(this);
8-
this._checkForOutsideClick = this._checkForOutsideClick.bind(this);
9-
this._checkForChildrenClick = this._checkForChildrenClick.bind(this);
10-
}
11-
12-
connectedCallback() {
13-
super.connectedCallback();
14-
// We listen for focusin(instead of foxus), because it bubbles and gives the right event order
15-
window.addEventListener('focusin', this._setTouchedAndPrefilled);
16-
17-
document.addEventListener('click', this._checkForOutsideClick);
18-
this.addEventListener('click', this._checkForChildrenClick);
19-
20-
// checks for any of the children to be prefilled
21-
this._checkboxGroupPrefilled = super.prefilled;
22-
}
23-
24-
disconnectedCallback() {
25-
super.disconnectedCallback();
26-
window.removeEventListener('focusin', this._setTouchedAndPrefilled);
27-
document.removeEventListener('click', this._checkForOutsideClick);
28-
this.removeEventListener('click', this._checkForChildrenClick);
29-
}
30-
31-
get touched() {
32-
return this._checkboxGroupTouched;
33-
}
34-
35-
/**
36-
* Leave event will be fired when previous document.activeElement
37-
* is inside group and current document.activeElement is outside.
38-
*/
39-
_setTouchedAndPrefilled() {
40-
const groupHasFocus = this.focused;
41-
if (this.__groupHadFocus && !groupHasFocus) {
42-
this._checkboxGroupTouched = true;
43-
this._checkboxGroupPrefilled = super.prefilled; // right time to reconsider prefilled
44-
this.__checkboxGroupPrefilledHasBeenSet = true;
45-
}
46-
this.__groupHadFocus = groupHasFocus;
47-
}
48-
49-
_checkForOutsideClick(event) {
50-
const outsideGroupClicked = !this.contains(event.target);
51-
if (outsideGroupClicked) {
52-
this._setTouchedAndPrefilled();
53-
}
54-
}
55-
56-
// Whenever a user clicks a checkbox, error messages should become visible
57-
_checkForChildrenClick(event) {
58-
const childClicked = this._childArray.some(c => c === event.target || c.contains(event.target));
59-
if (childClicked) {
60-
this._checkboxGroupTouched = true;
61-
}
62-
}
63-
64-
get _childArray() {
65-
// We assume here that the fieldset has one set of checkboxes/radios that are grouped via attr
66-
// name="groupName[]"
67-
const arrayKey = Object.keys(this.formElements).filter(k => k.substr(-2) === '[]')[0];
68-
return this.formElements[arrayKey] || [];
69-
}
70-
714
// eslint-disable-next-line class-methods-use-this
725
__isRequired(modelValues) {
736
const keys = Object.keys(modelValues);

‎packages/checkbox-group/stories/index.stories.js‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { storiesOf, html } from '@open-wc/demoing-storybook';
33
import '../lion-checkbox-group.js';
44
import '@lion/checkbox/lion-checkbox.js';
55
import '@lion/form/lion-form.js';
6+
import { localize } from '@lion/localize';
67

78
storiesOf('Forms|Checkbox Group', module)
89
.add(
@@ -123,4 +124,58 @@ storiesOf('Forms|Checkbox Group', module)
123124
</form></lion-form
124125
>
125126
`;
127+
})
128+
.add('Validation 2 checked', () => {
129+
const hasMinTwoChecked = value => {
130+
const selectedValues = value['scientists[]'].filter(v => v.checked === true);
131+
return {
132+
hasMinTwoChecked: selectedValues.length >= 2,
133+
};
134+
};
135+
localize.locale = 'en-GB';
136+
try {
137+
localize.addData('en-GB', 'lion-validate+hasMinTwoChecked', {
138+
error: {
139+
hasMinTwoChecked: 'You need to select at least 2 values',
140+
},
141+
});
142+
} catch (error) {
143+
// expected as it's a demo
144+
}
145+
146+
const submit = () => {
147+
const form = document.querySelector('#form');
148+
if (form.errorState === false) {
149+
console.log(form.serializeGroup());
150+
}
151+
};
152+
return html`
153+
<lion-form id="form" @submit="${submit}"
154+
><form>
155+
<lion-checkbox-group
156+
name="scientistsGroup"
157+
label="Who are your favorite scientists?"
158+
help-text="You should have at least 2 of those"
159+
.errorValidators=${[[hasMinTwoChecked]]}
160+
>
161+
<lion-checkbox
162+
name="scientists[]"
163+
label="Archimedes"
164+
.choiceValue=${'Archimedes'}
165+
></lion-checkbox>
166+
<lion-checkbox
167+
name="scientists[]"
168+
label="Francis Bacon"
169+
.choiceValue=${'Francis Bacon'}
170+
></lion-checkbox>
171+
<lion-checkbox
172+
name="scientists[]"
173+
label="Marie Curie"
174+
.choiceValue=${'Marie Curie'}
175+
></lion-checkbox>
176+
</lion-checkbox-group>
177+
<button type="submit">Submit</button>
178+
</form></lion-form
179+
>
180+
`;
126181
});

‎packages/checkbox-group/test/lion-checkbox-group.test.js‎

Lines changed: 1 addition & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { expect, html, fixture, triggerFocusFor, nextFrame } from '@open-wc/testing';
2-
import sinon from 'sinon';
1+
import { expect, html, fixture, nextFrame } from '@open-wc/testing';
32

43
import { localizeTearDown } from '@lion/localize/test-helpers.js';
54

@@ -11,87 +10,6 @@ beforeEach(() => {
1110
});
1211

1312
describe('<lion-checkbox-group>', () => {
14-
// Note: these requirements seem to hold for checkbox-group only, not for radio-group (since we
15-
// cannot tab through all input elements).
16-
17-
it(`becomes "touched" once the last element of a group becomes blurred by keyboard
18-
interaction (e.g. tabbing through the checkbox-group)`, async () => {
19-
const el = await fixture(`
20-
<lion-checkbox-group>
21-
<label slot="label">My group</label>
22-
<lion-checkbox name="myGroup[]" label="Option 1" value="1"></lion-checkbox>
23-
<lion-checkbox name="myGroup[]" label="Option 2" value="2"></lion-checkbox>
24-
</lion-checkbox-group>
25-
`);
26-
await nextFrame();
27-
28-
const button = await fixture(`<button>Blur</button>`);
29-
30-
el.children[1].focus();
31-
expect(el.touched).to.equal(false, 'initially, touched state is false');
32-
el.children[2].focus();
33-
expect(el.touched).to.equal(false, 'focus is on second checkbox');
34-
button.focus();
35-
expect(el.touched).to.equal(
36-
true,
37-
`focus is on element behind second checkbox
38-
(group has blurred)`,
39-
);
40-
});
41-
42-
it(`becomes "touched" once the group as a whole becomes blurred via mouse interaction after
43-
keyboard interaction (e.g. focus is moved inside the group and user clicks somewhere outside
44-
the group)`, async () => {
45-
const groupWrapper = await fixture(`
46-
<div tabindex="0">
47-
<lion-checkbox-group>
48-
<label slot="label">My group</label>
49-
<lion-checkbox name="myGroup[]" label="Option 1" value="1"></lion-checkbox>
50-
<lion-checkbox name="myGroup[]" label="Option 2" vallue="2"></lion-checkbox>
51-
</lion-checkbox-group>
52-
</div>
53-
`);
54-
await nextFrame();
55-
56-
const el = groupWrapper.children[0];
57-
await el.children[1].updateComplete;
58-
el.children[1].focus();
59-
expect(el.touched).to.equal(false, 'initially, touched state is false');
60-
el.children[2].focus(); // simulate tab
61-
expect(el.touched).to.equal(false, 'focus is on second checkbox');
62-
// simulate click outside
63-
sinon.spy(el, '_setTouchedAndPrefilled');
64-
groupWrapper.click(); // blur the group via a click
65-
expect(el._setTouchedAndPrefilled.callCount).to.equal(1);
66-
// For some reason, document.activeElement is not updated after groupWrapper.click() (this
67-
// happens on user clicks, not on imperative clicks). So we check if the private callbacks
68-
// for outside clicks are called (they trigger _setTouchedAndPrefilled call).
69-
// To make sure focus is moved, we 'help' the test here to mimic browser behavior.
70-
// groupWrapper.focus();
71-
await triggerFocusFor(groupWrapper);
72-
expect(el.touched).to.equal(true, 'focus is on element outside checkbox group');
73-
});
74-
75-
it(`becomes "touched" once a single element of the group becomes "touched" via mouse interaction
76-
(e.g. user clicks on checkbox)`, async () => {
77-
const el = await fixture(`
78-
<lion-checkbox-group>
79-
<lion-checkbox name="myGroup[]"></lion-checkbox>
80-
<lion-checkbox name="myGroup[]"></lion-checkbox>
81-
</lion-checkbox-group>
82-
`);
83-
await nextFrame();
84-
85-
el.children[1].focus();
86-
expect(el.touched).to.equal(false, 'initially, touched state is false');
87-
el.children[1].click();
88-
expect(el.touched).to.equal(
89-
true,
90-
`focus is initiated via a mouse event, thus
91-
fieldset/checkbox-group as a whole is considered touched`,
92-
);
93-
});
94-
9513
it('can be required', async () => {
9614
const el = await fixture(html`
9715
<lion-checkbox-group .errorValidators=${[['required']]}>

‎packages/field/src/FormControlMixin.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ export const FormControlMixin = dedupeMixin(
495495
* an error message shouldn't be shown either.
496496
*
497497
*/
498-
return (this.touched && this.dirty && !this.prefilled) || this.prefilled || this.submitted;
498+
return (this.touched && this.dirty) || this.prefilled || this.submitted;
499499
}
500500

501501
// aria-labelledby and aria-describedby helpers

‎packages/field/src/FormatMixin.js‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/* eslint-disable class-methods-use-this */
22

33
import { dedupeMixin } from '@lion/core';
4-
import { ObserverMixin } from '@lion/core/src/ObserverMixin.js';
54
import { Unparseable } from '@lion/validate';
65

76
// For a future breaking release:
@@ -50,7 +49,7 @@ import { Unparseable } from '@lion/validate';
5049
export const FormatMixin = dedupeMixin(
5150
superclass =>
5251
// eslint-disable-next-line no-unused-vars, no-shadow
53-
class FormatMixin extends ObserverMixin(superclass) {
52+
class FormatMixin extends superclass {
5453
static get properties() {
5554
return {
5655
/**
@@ -121,13 +120,24 @@ export const FormatMixin = dedupeMixin(
121120
};
122121
}
123122

124-
static get syncObservers() {
125-
return {
126-
...super.syncObservers,
127-
_onModelValueChanged: ['modelValue'],
128-
_onSerializedValueChanged: ['serializedValue'],
129-
_onFormattedValueChanged: ['formattedValue'],
130-
};
123+
_requestUpdate(name, oldVal) {
124+
super._requestUpdate(name, oldVal);
125+
126+
if (name === 'modelValue' && this.modelValue !== oldVal) {
127+
this._onModelValueChanged({ modelValue: this.modelValue }, { modelValue: oldVal });
128+
}
129+
if (name === 'serializedValue' && this.serializedValue !== oldVal) {
130+
this._onSerializedValueChanged(
131+
{ serializedValue: this.serializedValue },
132+
{ serializedValue: oldVal },
133+
);
134+
}
135+
if (name === 'formattedValue' && this.formattedValue !== oldVal) {
136+
this._onFormattedValueChanged(
137+
{ formattedValue: this.formattedValue },
138+
{ formattedValue: oldVal },
139+
);
140+
}
131141
}
132142

133143
/**

‎packages/field/src/InteractionStateMixin.js‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { dedupeMixin } from '@lion/core';
2-
import { ObserverMixin } from '@lion/core/src/ObserverMixin.js';
32
import { Unparseable } from '@lion/validate';
43

54
/**
@@ -15,7 +14,7 @@ import { Unparseable } from '@lion/validate';
1514
export const InteractionStateMixin = dedupeMixin(
1615
superclass =>
1716
// eslint-disable-next-line no-unused-vars, no-shadow
18-
class InteractionStateMixin extends ObserverMixin(superclass) {
17+
class InteractionStateMixin extends superclass {
1918
static get properties() {
2019
return {
2120
/**
@@ -45,12 +44,15 @@ export const InteractionStateMixin = dedupeMixin(
4544
};
4645
}
4746

48-
static get syncObservers() {
49-
return {
50-
...super.syncObservers,
51-
_onTouchedChanged: ['touched'],
52-
_onDirtyChanged: ['dirty'],
53-
};
47+
_requestUpdate(name, oldVal) {
48+
super._requestUpdate(name, oldVal);
49+
if (name === 'touched' && this.touched !== oldVal) {
50+
this._onTouchedChanged();
51+
}
52+
53+
if (name === 'dirty' && this.dirty !== oldVal) {
54+
this._onDirtyChanged();
55+
}
5456
}
5557

5658
static _isPrefilled(modelValue) {

0 commit comments

Comments
 (0)